[FFmpeg-devel] [PATCH/RFC] Fix dvbsub framing inconsistencies/add dvbsub bsf

[Michael Niedermayer]

On Tue, Sep 28, 2010 at 01:41:11PM +0200, Michael Niedermayer wrote:
> On Tue, Sep 28, 2010 at 01:34:08PM +0200, Tomas H?rdin wrote:
> > On Sun, 2010-09-26 at 04:37 +0200, Michael Niedermayer wrote:
> > > On Fri, Sep 24, 2010 at 04:28:45PM +0200, Tomas H?rdin wrote:
> > > > Hi
> > > > 
> > > > As you may know from IRC and my last thread on this list, I'm working on
> > > > fixing mpegts subtitle remuxing. The extradata problem was easy enough
> > > > to fix, but while writing the bitstream filter I discovered some
> > > > inconsistencies regarding the framing bytes surrounding the data the
> > > > decoder wants.
> > > > 
> > > > Since I don't have the specs at hand, I have been figuring this out by
> > > > looking at the code and hex dumps, and testing my results in ffplay and
> > > > vlc.
> > > > 
> > > > I'll be using a PES packet containing 14 B of subtitle essence data as
> > > > an example. It looks like this:
> > > > 
> > > > 00 00 01 BD 00 19 85 80 05 21 01 AB 9D CD
> > > > 20 00 0F 10 00 02 00 02 01 94 0F 80 00 02 00 00 FF
> > > > 
> > > > That second line (20 00 ... FF) is what the demuxer outputs. That gets
> > > > stripped by dvbsub_parser to the fourteen bytes below:
> > > > 
> > > > 0F 10 00 02 00 02 01 94 0f 80 00 02 00 00
> > > > 
> > > > This is the format that the decoder expects (starts with 0x0F). However,
> > > > this is not what the dvbsub encoder outputs. See dvbsub.c lines 211 and
> > > > 387
> > > > 
> > > >     *q++ = 0x00; /* subtitle_stream_id */
> > > > 
> > > >     /* page composition segment */
> > > > 
> > > >     *q++ = 0x0f; /* sync_byte */
> > > >     *q++ = 0x10; /* segment_type */
> > > > 
> > > > ...
> > > > 
> > > >     *q++ = 0xff; /* end of PES data */
> > > > 
> > > > In other words, the output from the encoder can not be fed into the
> > > > decoder. It also does not match what is output by the parser, but you
> > > > could also say that the parser's output does not match the encoder's.
> > > > 
> > > > Since the output from the encoder is typically fed straight to a muxer,
> > > > the missing 0x20 byte needs to be written before the data is written.
> > > > The hackish code relating to private_code in mpegtsenc.c takes care of
> > > > this:
> > > > 
> > > >     if (st->codec->codec_type == AVMEDIA_TYPE_SUBTITLE) {
> > > >         private_code = 0x20;
> > > >     }
> > > > 
> > > > ...
> > > > 
> > > >     if (private_code != 0)
> > > >         *q++ = private_code;
> > > > 
> > > > As I understand it, there are a number of ways to fix this:
> > > > 
> > > > 1. Add a bitstream filter that adds a 0x00 to the start and 0xFF to the
> > > > end of the subtitle packets
> > > > 2. Make the encoder not add said bytes and require its output to be fed
> > > > to a bitstream filter that adds 20 00 .. FF (to which the parsed packets
> > > > are also fed). This allows simplifying the muxer (remove the
> > > > private_code stuff)
> > > > 3. Patch the decoder to accept leading null bytes, and have the parser
> > > > only strip the leading 0x20 byte
> > > > 4. Remove the parser and the muxer hack and have the encoder output
> > > > packets with the 20 00 .. FF framing and accept them in the decoder
> > > > 
> > > > I am unsure which of these solution would be most suitable. Maybe
> > > > there's a better one?
> > > 
> > > id like to know first what these bytes are and if they belong into the
> > > codec, muxer or some intermediate layer.
> > 
> > Look at page 19 in EN 300 743 (section 7.1 - Syntax and semantics of the
> > PES data field for subtitling):
> > 
> >         When carrying a DVB subtitle stream, the PES_packet_data_bytes
> >         shall be encoded as the PES_data_field defined in
> >         the table below.
> >         
> >         PES_data_field() {
> >           data_identifier
> >           subtitle_stream_id
> >           while nextbits() == '0000 1111' {
> >              Subtitling_segment()
> >           }
> >           end_of_PES_data_field_marker
> >         }
> >         
> >         Semantics:
> >         
> >         data_identifier: For DVB subtitle streams the data_identifier
> >         field shall be coded with the value 0x20.
> >         
> >         subtitle_stream_id: This identifies the subtitle stream in this
> >         PES packet. A DVB subtitling stream shall be identified
> >         by the value 0x00.
> >         
> >         end_of_PES_data_field_marker: An 8-bit field with fixed contents
> >         '1111 1111'.
> 
> This sounds like these bytes should be added by the mpeg muxer and removed by
> the demuxer

after rereading this again, i think its not clear and this is likely why
we have the mess we do in svn
iam fine with handling these 3 byte fully inside the decoder & encoder instead
of the muxer/demuxer.
the spec is ambigously worded, its not clear if this is PES specific or
not


[...]
--
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://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100928/1afe3976/attachment.pgp>