[FFmpeg-devel] Collection of patches

Michael Niedermayer michaelni
Wed Apr 23 13:25:41 CEST 2008


On Wed, Apr 23, 2008 at 09:15:40AM +0200, Thorsten Jordan wrote:
[...]
> >> Do not split the h264 "headers" off too early, as when a SEI follows a
> >> SPS, split must not stop there but also include the following PPS:
> >> h264_parser_SEI_ignore.patch
> > 
> > This is IIRC wrong, correct solution was discussed somewhere on the
> > mailinglist.
> hmm i haven't searched yet, but when you rip the header of to e.g. write
> h264 data to MP4, the split function of the parser is used, and this
> doesn't rip off the PPS without the patch. The generated MP4 files didnt
> work. Maybe it has been fixed differently meanwhile, but i can't
> imaginge how.

Your patch generates invalid/broken h264. The current code generates broken
ones too but they dont work which prevents them from spreading. Thus your
patch clearly makes it worse, the issue is not fixed it just appears that it
is. The spliting API in ffmpeg has to be changed to allow spliting random
subsets of the headers out. Please see the mailing list where i described
this more precissely IIRC.


[...]
> >> undef DEBUG_SEEK in libavformat/utils.c as this causes much computations
> >> when opening large TS files (complete parsing of whole file on open
> >> instead on demand) - this was once rejected because it would break
> >> regression checks - I propose to adapt the check rather or to see why
> >> the check won't work when debug code is disabled! :
> >> proposal-undef-DEBUG_SEEK.patch
> > 
> > Why do the regression tests break with that?
> Initially submitted on 10/15/2007, you said first "patch ok", Benoit
> Fouet applied it, then he replied on 10/16/2007 "this breaks seek
> regression test (i missed that) as follows, what should
> be done ?" with the following text:
[...]
>  ret: 0 st: 1 dts:0.000000 pts:0.000000 pos:31790 size:278 flags:1
> -ret: 0 st:-1 ts:1.894167 flags:1
> -ret: 0 st: 1 dts:0.975000 pts:0.975000 pos:355089 size:278 flags:1
> +ret:-1 st:-1 ts:1.894167 flags:1
>  ret: 0 st: 0 ts:0.788000 flags:0

Find out why this changes and explain it to us, until then the patch is
rejected.


[...]
> 
> I wrote at that time that it caused much seeking when opening a file
> because of the debug code that causes index generation when not needed.
> With that patch one can open large TS/PS files and indices are generated
> on demand, which works nicely. Why would one want to lose that great
> feature because of some test, that may be wrong? I dont have the
> knowledge of the testcode to change it, or else i would have proposed
> that too.

The regression tests have a purpose, that is to catch regressions, every
change to the values must be fully understood or the change will not happen
in ffmpeg svn.

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Observe your enemies, for they first find out your faults. -- Antisthenes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080423/a78cd787/attachment.pgp>



More information about the ffmpeg-devel mailing list