On Dienstag, 26. Oktober 2010, Monty Montgomery wrote:
> > - 8aa33e3e Rwrite the latency timing/calculation for the OSS backend
> >  There is parctically no explanation why the change is good and why
> >  a new mutex is needed. It looks dangerous. How extensively has the
> >  code been excercised? Why is a change needed at all? I am not aware
> >  of people complaining that the OSS driver does not work. But I
> >  might be missing something.
>
> The OSS latency calculation in 2.1CV is completely nonfunctional on
> the native OSS, OSS-via-ALSA-emu and OSS-via-ALSA-emu-via-Pulse emu
> machines here. The OSS driver only 'works' when the driver-based
> latency calculation is disabled (the 'enable software sync'
> preferences box, which doesn't enable anything-- it disables driver
> latency polling and has playback 'fly blind').
>
> People are, in general, used to sync always being broken. :-)

OK.

Speaking of sync: With the patches applied, I get acceptable playback only 
when I "Disable hardware synchronization". Otherwise, although audio output 
is continuous, video is jerky at the beginning, and then catches up with the 
audio. This happens at every playback change (forward, reverse, half-speed, 
normal-speed, double-speed).

> > - e0569663 Actually check for error return codes in OSS
> >  This does not adhere to the coding standard that is used in surrounding
> >  code. Fixing that would take too much of my time. Remember that it is
> >  good tone to copy the coding style that you find rather than to force
> >  your own style onto existing code. This includes white-space style.
> >  (IOW, please use tabs, not 8 spaces.)
>
> My emacsen here are actually forced to use spaces and two-space
> indent.  Not sure how tabs ended up in it.
>
> As for stylistic 'forcing' in most projects here at least, we've long
> since abandoned the formatting war over here.  He who writes the code
> gets to format it, and for the most part we loosely follow the
> surrounding style.  But insisting on whitespace-perfect style
> emulation... urk. Or is this about braces? :-)

Actually, it is about both.

What if I have to fix your code? My formating is different from yours again. 
It's going to make a an unacceptable mess.

Please find out how to set the style for Emacs. It's certainly possible. There 
are lots of projects out there where Emacs is used to edit code that is not 
two-space indented.

> > - 64e8022f Eliminate the YUV file loader's wont
> >  Before I merge this change, I'd like to know why the patch solves a
> >  problem (and what the problem exactly is). "It happens more often
> >  than you think" is a bit too much hand-waving. Can I easily reproduce
> >  the bug? (Oh, and the commant about coding style applies, too.)
>
> The YUV4MPEG2 loader was building a from-scratch transient index that
> required reading every byte in the raw file on every call to open.
> Open is called *often*, not just when the project is loaded.  If the
> loader already had an index, it threw it away.  It was all in memory,
> nothing was saved. This means that, for example, every undo required
> every y4m file in the project to rebuild indexes from scratch.  On the
> movie I was working on, that was 20 minutes as it re-read just short
> of a terabyte of video...
>
> ...for no reason.  The index was useless, every frame is the same
> size.  The y4m spec doesn't allow frame size deviation (that includes
> frame headers).
>
> I realize this is all still to terse; I'll elaborate in more detail on
> each point once I've gotten some sleep.

Thanks, it's appreciated. One *important* piece of information is that this is 
about YUV4MPEG2 loader, not about any sort of YUV-like frames. This restricts 
the scope of the patch quite a lot and makes it less critical.

BTW, when you formulate a commit message, please begin it with a single 
summary line (max 76 characters), followed by a blank line, followed by the 
justification. The justification should be written with an audience in mind 
that is at most half-aware of what is going on in the subsystem that you are 
changing. You should explain what the subsystem _should_ do, what it actually 
does (i.e. the reason of the bug), and how to reproduce the problem. If the 
way how you fix a bug is non-obvious, it should be explained as well.

-- Hannes

_______________________________________________
Cinelerra mailing list
Cinelerra@skolelinux.no
https://init.linpro.no/mailman/skolelinux.no/listinfo/cinelerra

Reply via email to