I demand that Morfsta may or may not have written...

> Attached is a patch to enable decoding of H264 video streams using the
> CoreAVC Win32 DLL, the latest HG clone of xine-lib and Reinhard's vdr-xine
> and of course VDR. CoreAVC is a commercial and fast software-based H264
> decoder.
> Note, to increase performance I have disabled the xine de-interlacer
> for H264 streams as by default the CoreAVC deinterlacer is enabled. I
> don't think it's as good as the xine de-interlacer, but if you want to
> try re-enable xine deint on your CPU [...]

If at all possible, the CoreAVC deinterlacer should be made useable as a
deinterlacer plugin, much as tvtime and pp are.

> Note, I haven't written the majority of this patch, it has been pulled
> together from about 4 other patches (budice's work on DVBN and the google
> code site itself).

If you can untangle them and set up a repository containing the individual
patches *or* make a quilt series from them, that'd be better. It might be
worth using From: and Subject: headers at the top (RFC2822-style) for proper
attribution ("hg import" understands these).

> I have added code to automatically detect the size of the H264 stream and
> to set the aspect ratio of the resultant picture as well as making it apply
> cleanly (hopefully!)  to today's HG of xine.

Without overriding the user's preference, I hope.

> Therefore, the code is nothing more than a total hack and will not likely
> see inclusion in xine in it's current form.

You're right (and you mean "its").

(! = will cause rejection; ? - may cause rejection; - = should be fixed.)

! New files are missing copyright/licence headers.

- C++ comments. (We prefer C comments.)

- printf usage - you want xprintf or lprintf.

? static SampleProcUserData sampleProcData - why is this not part of an
  instance-specific struct, or allocated as needed?

- #include "xine/..." - should use <>, not ""

- White-space and {} changes which don't change the meaning of the code don't
  belong in the patch.

- 32-bit assumptions. Probably safe for now...

? Use of signal() - be careful: that could trample all over the front end's
  own signal handling. (I note that signal(2) says that its effects in a
  multi-threaded process is undefined...)

? src/libw32dll/wine/win32.c, lines 5445-5454: fixed-size buffer & sprintf.
  Are you *sure* that there's no chance of an overflow here?

- Some indentation/line-break fixups where code is already changed would be
  good. :-)

[snip already-known problems etc.]
| Darren Salt    | linux or ds at              | nr. Ashington, | Toon
| RISC OS, Linux | youmustbejoking,demon,co,uk | Northumberland | Army
| + Output less CO2 => avoid massive flooding.    TIME IS RUNNING OUT *FAST*.

Why are there no tan M&M's?

vdr mailing list

Reply via email to