[FFmpeg-devel] BUG in use of extradata and extradata_size with dvb subtitles and teletext

[Andriy Lysnevych]

Unfortunately I have no example of multiple DVB subtitles in one stream. We
have only example of multiple DVB teletext in one stream that is very
similar in logic to DVB subtitles.

Finally I found .mkv file with DVB subtitles and examined it. Now I see
that our patch is wrong and generates wrong DVB subtitles payload for
matroska container.

But event without our patch DVB subtitle encoder generates incorrect DVB
subtitles payload for matroska. It generates leading 0x00 that is ok for TS
muxer but Matroska requires DVB subtitles without leading 0x00.

So what I see currently in FFmpeg:

1) We have two different DVB subtitles AVPacket.data formats internally in
FFmpeg that is very bad and lead to bugs.
2) Copy of DVB subtitles from TS muxer to TS demuxer broken because of
point 1)
3) DVB subtitles encoder produces correct payload for TS muxer. But this
payload is incorrect for Matroska muxer. Thus DVB subtitles encoder ->
Matroska muxer chain is broken.
4) Muxing DVB teletext is broken in TS muxer because it incorrectly adds
0x20 to beginning of DVB teletext payload. This 0x20 should be added to DVB
subtitles only.

All of this should be fixed in one patch because all 4 points are very
interconnected.

Also I have questions about extradata:
If a developer that uses FFmpeg library generates TS stream he need create
this extradata and add to DVB subtitles stream manually? It means
developers have to know exact binary format of this extradata?

P.S.
Can you provide me with correct .nut file that contains DVB subtitles so I
can inspect it too?


On Tue, Dec 31, 2013 at 4:28 PM, JULIAN GARDNER <joolzg at btinternet.com>wrote:

>
>
>
>
>
>
>
> >________________________________
> > From: Andriy Lysnevych <andriy.lysnevych at gmail.com>
> >To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
> >Cc: serhii.marchuk at gmail.com
> >Sent: Tuesday, 31 December 2013, 10:23
> >Subject: Re: [FFmpeg-devel] BUG in use of extradata and extradata_size
> with dvb subtitles and teletext
> >
> >
> >Hi,
> >
> >Thanks for your comments.
> >
> >1) As I pointed before there are two different AVPacket.data formats of
> DVB
> >subtitles in FFmpeg. It is not obvious and causes developers to do bugs.
> >Because of this copy subtitles from TS demuxer to TS muxer does not work.
> >
> >>the reason it was failing in copy was due to the not copying of the ids
> >from the input stream to the output stream. >Thats all that was needed.
> >
> >We investigated the issue. Subtitles in TS stream should go in format
> "0x20
> >0x00 [subtitles data] 0xFF". Because TS demuxer generates AVPacket.data in
> >different format than TS muxer expects on output you get TS stream with
> >wrong subtitles in format: "0x20 0x00 [subtitles data]" (i.e. without 0xFF
> >at the end). That is why copy does not work. Making all DVB subtitles
> >internally to be in one format fixes the issue.
> >
>
> I think the 0x20 .... 0xff fix should be a seperate patch
>
> >> In addition there is another issue: as far as I can tell this patch
> >breaks backward-compatibility.
> >> So an application that uses its own MPEG-TS demuxer together with our
> DVB
> >subtitle decoder will break after this change.
> >
> >Yes our goal is to make all DVB subtitles across all FFmpeg components to
> >be in one format. It is not possible to do without breaking
> >backward-compatibility. Otherwise it will cause more bugs in future.
> >
> >2) Regarding matroska, nut and wtv:
> >
> >>I checked: matroska, nut and wtv all support DVB subtitles in FFmpeg.
> >>So I would expect that your patch misses changes for all 3 of those
> >>(though I have a suspicion that the exact formats those use differs
> >>slightly so that at least stream copy from those into MPEG-TS might not
> >>work well either way).
> >
> >As I see the same DVB subtitles parser is used for all matroska, nut, TS
> >and wtv. The parser expects DVB subtitles to be in format "0x20 0x00
> >[subtitles data] 0xFF":
> >
> >if (buf_size < 2 || buf[0] != 0x20 || buf[1] != 0x00) {
> >    av_dlog(avctx, "Bad packet header\n");
> >    return -1;
> >}
> >
> >...
> >
> >else if (*p == 0xff)
> >
> >...
> >
> >From other hand I see that DVB subtitles encoder generates subtitles in
> >format "0x00 [subtitles data] 0xFF" (without leading 0x20).
> >
> >Only MPEG-TS muxer has "hack" that adds leading 0x20 before each DVB
> >subtitle payload (actually it incorrectly adds 0x20 to teletext also and
> >that is why muxing teletext is broken too). Nut and matroska do not add
> >0x20 before payload to DVB subtitles encoder (can't find this in code). As
> >the result muxing DVB subtitles into matroska and nut is currently broken.
> >At least data expected by parser and data generated by muxers is
> different.
> >Please correct me if I am wrong.
> >
> >I can't find documentation how DVB subtitles should be stored in matroska
> >or nut. Also I can't find real samples to check it. Please share
> >documentation or samples with us if you have them.
>
> >
>
> >Our patch fixes subtitle muxing in matroska and nut. As the result all
> >subtitles will be in format: "0x20 0x00 [subtitles data] 0xFF" even in nut
> >and matroska.
> >
> >About metadata: as a developer I need ability to specify DVB subtitles
> >parameters for TS muxer via FFmpeg API to correctly generate PMT of TS
> >stream. Also I need ability to read/copy the same parameters from TS
> >demuxer.
> >If metadata is not the place to store these values please advice better
> way
> >to do it.
> >
>
>
> I still think the extradata way is the way we should fix this and also
> allow multiple values per pid, not limit it to 4 bytes which it currently
> does.
>
> Can you make the stream you have available with multiplte dvb streams?
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>