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

[Tomas Härdin]

On Tue, 2010-09-28 at 15:18 +0200, Michael Niedermayer wrote:
> 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

Considering no other codecs have any such special handling, doing it in
the decoder/encoder seems reasonable. The patch I posted yesterday does
exactly this, though it is a little hackish.

/Tomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100929/f9125b8b/attachment.pgp>