[FFmpeg-devel] what to do when a patch breaks regression test

[Michael Niedermayer]

On Thu, Feb 09, 2012 at 05:45:08PM -0700, Pavel Koshevoy wrote:
> Hi,
> 
> I am trying to solve an issue JW Player is having with TS fragments
> produced by httpsegmenter -- http://code.google.com/p/httpsegmenter/source/browse/trunk/segmenter.c
> 
> Essentially, JW Player can't handle fragments that do not start with
> a PAT/PMT packet.  Currently TS muxer outputs PAT/PMT periodically
> every 40 packets.  What I did was to force it to output PAT/PMT on
> every video keyframe as well.  I think this change is justified
> because of this comment in mpegtsenc.c --  /* Arbitrary values,
> PAT/PMT could be written on key frames */.
> 
> Of course this breaks the TS muxer regression test:
> 
> TEST    lavf-swf
> TEST    lavf-tga
> TEST    lavf-tiff
> TEST    lavf-ts
> --- /Users/pkoshevoy/src/ffmpeg/tests/ref/lavf/ts       2012-02-09
> 13:39:47.000000000 -0700
> +++ tests/data/fate/lavf-ts     2012-02-09 17:29:41.000000000 -0700
> @@ -1,3 +1,3 @@
> -8f61dad7652abbab9e49fca274dabf30 *./tests/data/lavf/lavf.ts
> +34f95a300355d474767b436430eba15b *./tests/data/lavf/lavf.ts
>  406644 ./tests/data/lavf/lavf.ts
>  ./tests/data/lavf/lavf.ts CRC=0x133216c1
> Test lavf-ts failed. Look at tests/data/fate/lavf-ts.err for details.
> make: *** [fate-lavf-ts] Error 1
> 
> 
> What should I do now?  Do I need to update the test?  If yes -- how?

see tests/ref/lavf/ts


> Will my patch be accepted into master?

If no problems are found, yes, the idea sounds solid


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Rewriting code that is poorly written but fully understood is good.
Rewriting code that one doesnt understand is a sign that one is less smart
then the original author, trying to rewrite it will not make it better.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120210/ee36ecd5/attachment.asc>