[haiku-commits] Re: haiku: hrev52055 - src/kits/media headers/os/media

  • From: "Adrien Destugues" <pulkomandy@xxxxxxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Fri, 27 Jul 2018 09:13:51 +0000

27 juillet 2018 09:40 "Dario Casalinuovo" <b.vitruvio@xxxxxxxxx> a écrit:

OK. My initial mail in this thread wasn't directed at you, but I wanted
to draw attention to it from the person having done the commit. I have
no time to work on it either.

Did you consider, it is two weeks I try to draw attention to this commit?

You have commit access. Just revert the commit, it would waste a lot less time.
You can explain the reasons in the commit message and we can happily move on and
rework and resubmit the patch.

If you look at WebKit git log, for example, you will see that about half of it
is reverting things, over and over again. There is no need to go on a quest on
the mailing list to find someone else who will do the revert for you.


I spent a great amount of time, more than you can imagine, digging in
the media_kit and fixed various long standing issues.

Just out of curiosity, are you aware that I worked on MediaKit before
you? I could say the exact same thing about myself, but why is it even
relevant here? I am thinking, when you suddenly tell people how much you
have worked on something, it isn't necessarily an argument in favor of
whatever your opinion is in some random discussion. It just looks like
"I don't want to waste time actually convincing someone with real
arguments." Sometimes, it is useful to get done quicker, but here? What
are you even trying to say?

I will tell the thing here: I think we should take the general design of the 
media_kit but rewrite
it from scratch.

You each time come in and say that media_kit is perfect, BMediaEventLooper is 
the best and so on.

Here is my view on it:
- It is certainly not perfect. The API is not easy to use, not all that well 
documented despite the
  efforts from Be days, and few people actually know all parts of it.
- However, we have a mostly working implementation, and it works well enough 
for now, so I prefer to
  focus my efforts in other parts of the code (intel_extreme driver, ffmpeg 
decoder, webkit, etc)
  and live with an imperfect API for R1.

It is not a matter of wether the API is great or not, but what to focus on 
right now.

As a result, I did not do much research in the state of the current API. You 
did, so it is up to
you to document the limitations, and then the possible solutions. From the 
discussions I followed,
you have documented your planned solutions, but communication between us failed 
at some point and
I missed the part where you listed the limitations and why your new design 
fixes them.

Of course this is much easier to do in real life. I could interrupt you as you 
explain things to
me, to ask for clarifications on one or another point. So I would suggest to 
wait a little, get
R1 out with the current API, and then we can start discussing the new API. 
There are some things
I don't agree with in your new proposed design, but then again I have little 
experience on the
media API. I do have however more experience on the codecs side and the ffmpeg 
plugin, as well as
the use of game kit classes (and the limitations and bugs we have on that side 
as well).


Even this discussion, about media_format is obviously related to the fact we 
have to introduce
BMediaFormat based on a BMessage-like class using key,value primitives. Don't 
you want to consider
my concerns? Let it be. But I listen at you always as being a respectable 
programmer, but don't
come here and tell me that continuing to use a C function like memset in 
place of C++ constructs
(whatever construct, not necessarily my solution which Ingo indeed pointed 
out as sub-efficient) is
any good for the code quality of the project.

No one ever said it was good for the code quality. It slipped through review (I 
already mentionned this).
Now we have wasted several man-hours arguing over a single line of code, 
writing kilobytes of e-mail when
it would be so simple to rewrite the Clear method to initialize the union 
field-by-field to a value that
makes sense. Wouldn't that be simpler and more constructive?

I hate to quote Linus Thorvalds, but I have to again: "talk is cheap, show me 
the code" (which Korli did
by submitting a patch doing what you suggested: 
https://review.haiku-os.org/#/c/haiku/+/358/ - ultimately
it was rejected because, I agree with Ingo and Stippi, it is not more readable 
than a Clear method and
not as optimal). This leaves open the implementation of a Clear method that 
does the right thing, however.

-- 
Adrien.


Other related posts: