[FFmpeg-devel] [PATCH] DVBSUBTILES, fixes to decoding/encoding and a new way of burning the subtitles onto the screen

Mon Oct 3 16:44:25 CEST 2011

On Mon, Oct 03, 2011 at 03:33:24PM +0100, JULIAN GARDNER wrote:
[...]
> >On Mon, Oct 03, 2011 at 08:41:27AM +0100, JULIAN GARDNER wrote:
[...]
> >> >On Tue, Sep 27, 2011 at 08:24:24PM +0100, JULIAN GARDNER wrote:
> >> >> 
> >> >> 
> >> >> ----- Original Message -----
> >> >> > From: Michael Niedermayer <michaelni at gmx.at>
> >> >> > To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
> >> >> > Cc: 
> >> >> > Sent: Tuesday, 27 September 2011, 5:12
> >> >> > Subject: Re: [FFmpeg-devel] [PATCH] DVBSUBTILES, fixes to decoding/encoding and a new way of burning the subtitles onto the screen
> >> >> > 
> >> >> > On Mon, Sep 26, 2011 at 11:43:10PM +0100, JULIAN GARDNER wrote:
> >> >> > [...]
> >> >> > 
> >> >> >>  Changes made to dvbsub*.c are to fix to the encoding and decoding, not real 
> >> >> > much to say but if you use the old source against the TS file i posted to the 
> >> >> > bug system months ago you will see that it did not work, it does now.
> >> >> > 
> >> >> > i tried with ticket/153/bbc_small.ts
> >> >> > playing it with ffplay works, but not pretty, the colors are off but
> >> >> > it works. applyimg your patch makes it worse, there are artifacts
> >> >> > at the right and left of the first "help" text and valgrind reports
> >> >> > below
> >> >> > 
> >> >> > ==31934== Use of uninitialised value of size 8
> >> >> > ==31934==    at 0x43B0EB: video_image_display (ffplay.c:533)
> >> >> > ==31934==    by 0x43F71D: main (ffplay.c:968)
> >> >> > ==31934==
> >> >> > ==31934== Use of uninitialised value of size 8
> >> >> > ==31934==    at 0x43B136: video_image_display (ffplay.c:539)
> >> >> > ==31934==    by 0x43F71D: main (ffplay.c:968)
> >> >> > ==31934==
> >> >> > ==31934== Use of uninitialised value of size 8
> >> >> > ==31934==    at 0x43B17E: video_image_display (ffplay.c:547)
> >> >> > ==31934==    by 0x43F71D: main (ffplay.c:968)
> >> >> > ==31934==
> >> >> > ==31934== Use of uninitialised value of size 8
> >> >> > ==31934==    at 0x43B205: video_image_display (ffplay.c:553)
> >> >> > ==31934==    by 0x43F71D: main (ffplay.c:968)
> >> >> > ==31934==
> >> >> > 
> >> >> 
> >> >> Attached is a new patch against my previous code which fixes the problem above, what is happening is that we have corrupted data and i was not waiting for a correct start of data. Patch also removes some old code and replaces it with calls to av_log_dump.
> >> >> 
> >> >> Hopefully this is usuable, i used 'git diff', if its no good let me know.
> >> >
> >> >please read the diff you submit, it must be free of unrelated changes
> >> >for a human to be able to review it. also it must be applyable by using
> >> >patch
> >> >neither is true: (see below)
> >> >
> >> >I suggest you learn about git add -p, git reset, and git commit
> >> >and use them to split the patches up make sure they are free of
> >> >unrelated changes and make sure they apply against git master
> >> >
> >> >
> >> >patching file ffmpeg.c
> >> >Hunk #1 FAILED at 162.
> >> >Hunk #2 succeeded at 1028 (offset -4 lines).
> >> >Hunk #3 FAILED at 1045.
> >> >Hunk #4 succeeded at 1067 (offset -4 lines).
> >> >Hunk #5 FAILED at 1084.
> >> >Hunk #6 succeeded at 1455 (offset -5 lines).
> >> >Hunk #7 succeeded at 1602 with fuzz 1 (offset -10 lines).
> >> >Hunk #8 succeeded at 1648 (offset -9 lines).
> >> >Hunk #9 succeeded at 1707 with fuzz 1 (offset -8 lines).
> >> >Hunk #10 succeeded at 1892 (offset 45 lines).
> >> >Hunk #11 succeeded at 1901 with fuzz 1 (offset 44 lines).
> >> >Hunk #12 FAILED at 1929.
> >> >Hunk #13 succeeded at 4336 (offset 54 lines).
> >> >4 out of 13 hunks FAILED -- saving rejects to file ffmpeg.c.rej
> >> >
> >> >patching file libavcodec/dvbsubdec.c
> >> >Hunk #3 FAILED at 593.
> >> >Hunk #4 succeeded at 790 (offset -1 lines).
> >> >Hunk #5 FAILED at 804.
> >> >Hunk #6 succeeded at 889 (offset -4 lines).
> >> >Hunk #7 succeeded at 935 (offset -4 lines).
> >> >Hunk #8 succeeded at 946 (offset -4 lines).
> >> >Hunk #9 succeeded at 1044 (offset -4 lines).
> >> >Hunk #10 succeeded at 1179 (offset -4 lines).
> >> >Hunk #11 FAILED at 1207.
> >> >Hunk #12 FAILED at 1219.
> >> >Hunk #13 succeeded at 1258 (offset -4 lines).
> >> >Hunk #14 succeeded at 1361 (offset -4 lines).
> >> >Hunk #15 succeeded at 1405 (offset -4 lines).
> >> >Hunk #16 succeeded at 1470 with fuzz 2 (offset -3 lines).
> >> >Hunk #17 succeeded at 1489 (offset -3 lines).
> >> >Hunk #18 succeeded at 1548 (offset -3 lines).
[...]
> >> OK i will see what i need to do to get these 5 lines of extra code added, to FIX the problem.
> >> 
> >> I cant see how the patch fails as it comes from my dev tree which is where the 1st large patch came from.
> >> 
> >> Any help here would be nice.
> >> 
> >> joolz
> >> ps if you have 2 minutes you can take the fixes for the main patches, just look for "start epoch", only 4 lines need modifying
> >
> >thanks, i applied the 3 hunks out of 18 but i dont see how they can fix the
> >corruption. Could you explain what causes the corruption?
> >
> >What they do is they skip the first subtitle, but if one would seek
> >they would not skip it and still lead to corruption. Also the original
> >code displayed the first subtitle without corruption just wrong colors
[...]

> What the code does is not try to do any display until it gets a new "epoch", the subtitle test stream you have has corruption at the beginning as it was a chopped ts. I will look into the real reason for the glitch but this is the correct way, wait for a "full" update, objects, regions, cluts etc before trying to use the data

someone could concatenate several choped ts streams or even create
invalid combinations of objects regions and cluts intentionally,
epoch is never reset thus once its 1 its not going to help anymore

the subtitle decoder should not end up using uninitialised memory
or worse crashing or even worse writing outside arrays
Thats what iam concerned about (and no ive not seen it do the later 2
just randomly changing corruption outside subtitles on the screen, so
at least used uninitialized memory was used)


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

it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- Aristotle
-------------- 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/20111003/14b2c75b/attachment.asc>