On 3/5/07, Allan N. Snider <[EMAIL PROTECTED]> wrote:
Francesco:
Thanks for the comments and compliments.
My pleasure.
- The filter, while well written, is not compliant with our STYLE
guidelines
(see transcode source root directory/STYLE).
Yes, I thought the STYLE issue would arise. Well, "If in Rome, do as
the Romans do", so I'll get it somewhat compliant. As a possible
compromise, could a set of "astyle" options be devised that would be
compliant with your style guidelines? (For example "astyle
--style=linux").
That would be a really nice thing to do, even if identation, bracket
position and other things that
can be automatically adjusted are just a little part of our STYLE
directives. For example,
identificators naming scheme is at least important as above, and that isn't
trivial to change using
a tool (AFAIK).
That would allow me to convert the code to my formatting tastes for
editing, (I'm an old timer, I get confused easily when the brackets aren't
where I like them),
then run "astyle" before submitting back. It might even be a good thing to
run the entire code base over with astyle. Get everything consistent.
I know, but unfortunately even here we have the same problems as above:
identation, spacig and bracket position are really the least of our problems
given our legacy codebase :|
- It will be better to add documentation for functions (see again STYLE)
used internally by filter, even if function does `trivial' things (in
that
case docs will be trivial too :) )
Yes, I agree. It's just a function of time...
No problem, just a reminder.
- the proposed filter uses old module system, but that isn't a big deal,
yet. If is possible, module private variables should be packed into a
struct (to make future transition to NMS easier without significant pain
today). And they must be made static!
I don't know the difference between the "old" and "new" module system,
but I will move all the globals into a struct. Everything will be made
static.
Fine.
What is NMS?
Sorry, I speaked transcodinsh here :) NMS stands for New Module System, the
new module schema that I and Andrew have thinkered and that we've slowly
introducing in CVS (port -and sanitize- all the modules is neither easy nor
quick as it can look).
Basic documentation about NMS can be found on docs/module-system-API.txt;
Feel free to mail me about any inconsistency or incomplete/unclear thing on
specification.
At the present time, I do want the filter to be able to run the analysis
automatically, so I agree with moving the functionality to
filter/yait_core.c (which allows a common yait.h).
That's very nice :)
However, I'm not sure I want to drop the tcyait executable. Being a
perfectionist (and control freak), I will want to scrutinize the analysis
for each particular encoding, and possibly re-run the analysis with
different options, or make manual corrections to both the log and ops file
to get it "just perfect". Again, I would not want to have to regenerate the
delta information each time. So, I would like to see tools/tcyait.c stay
(permanently), although, it would simply be a command line argument parser
and would use filter/yait_core.c for all its functionality.
No big deal, I'm fine in having tcyait in codebase.
How about this?
[...]
I think I like that. Thoughts?
I like it too.
Oops, forgot to add: I attach a reviewed version of filter_yait.c that
addresses most of STYLE issues outlined in the previous mail. Feel free to
drop it, flame it or take as a basis for further improvements.
Ok, I'll have a look at it. (I haven't yet).
Be warned: code was absolutely untested (as I don't own any NTSC videos)
PS: could you please elaborate the tcdecode bug mentioned on source?
This is a problem that has plagued me for some time. I'm not sure if
the problem is transcode itself, or in some support library used by
transcode. When specifying a frame range, transcode hangs at exit time, and
the filter close operation never gets called. [...] Does any of this ring
a bell? Anyway, that's why the flush was done in the filter, so that I lost
at most 5 frames when I had to kill transcode.
Apocalypse bell are ringing.
The problem is that our threading core -let's be honest- sucks. Badly. Is
outdated (it was designed
back in old linuxthreads days), the port was NPTL -that is much more
POSIXly- was pretty hackish,
and the whole threading design is questionable (single conditions on full
queues? sleep for synchronization? Ugh!). Debugging issues on this
infrastructure leads to severe headaches at best.
For 1.1.0 there is very little that can be improved. We are already really
late on release schedule
(mostly my bad, I'm sorry but spare time lacks[1] severely and fixing issues
without doing even more ugly hacks isn't a fast process), so threading core
will be left mostly untouched.
I'm reworking the decoder on those days carefully avoiding to touch too much
internals.
After 1.1.0 eventually out (or at least in featue freeze, and we're really
near), I finally bite the bullet
and I will propose a brand new threading organization.
[1] Even geeks have a job and a life too! :P
Have a look at tcyait.c when time provides,
Of course I'll do. I know little about NTSC but I'm interested on those
topics.
PPS:
I'm currently trying to complete my filter/extsub enhancements.
Unfortunately, I could not keep the functionality backwards compatible, so
decided to just rework the filter options as I saw fit. One can now specify
opacity and render colors for all three subtitle indices, anti-alias, zoom,
crop the subtitle image, etc. The question is, what do I call it now? I
can't replace the existing filter_extsub, or I'm going to break things (like
dvdrip), and I'm sure you don't want to see filter_extsub2.c (ever again),
so what can you suggest? extsub/filter_nextsub? (Next extsub, new
extsub)? It still should reside in the extsub directory as it uses the same
support code, subproc.c, etc.
nextsub is fine to me.
Bests,
--
Francesco Romani
