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

[Tomas Härdin]

On Thu, 2010-09-30 at 13:43 +0200, Tomas H?rdin wrote:
> On Wed, 2010-09-29 at 19:34 +0200, Reimar D?ffinger wrote:
> > On Tue, Sep 28, 2010 at 01:50:40PM +0200, Tomas H?rdin wrote:
> > > diff --git a/libavcodec/dvbsub.c b/libavcodec/dvbsub.c
> > > index ea55cae..08e05d4 100644
> > > --- a/libavcodec/dvbsub.c
> > > +++ b/libavcodec/dvbsub.c
> > > @@ -208,6 +208,7 @@ static int encode_dvb_subtitles(DVBSubtitleContext *s,
> > >      if (h->num_rects == 0 || h->rects == NULL)
> > >          return -1;
> > >  
> > > +    *q++ = 0x20; /* data_identifier */
> > >      *q++ = 0x00; /* subtitle_stream_id */
> > 
> > > @@ -1429,7 +1429,8 @@ static int dvbsub_decode(AVCodecContext *avctx,
> > >      if (buf_size <= 2)
> > >          return -1;
> > >  
> > > -    p = buf;
> > > +    //skip data_identifier and subtitle_stream_id
> > > +    p = buf + 2;
> > 
> > I guess it might be useful to set/query the subtitle_stream_id, though
> > I admit that's a separate issue.
> > But please add a space between // and the text.
> 
> Done
> 
> > Otherwise I think it's ok, however I suspect this change might cause
> > issue with applications that use their own demuxer with the dvb subtitle
> > decoder, no idea if that is likely to be a real issue and whether
> > we should try to do something about it...
> 
> For that case I made it check that the packet is in PES format, log an
> error and fail.
> 
> I also noticed the decoder failed due to the last check in the function.
> Adjusting p_end back one byte fixed that problem. That check should
> probably just print a warning instead of erroring out, or at least print
> an error instead of a debug message.
> 
> With the attached patch, the following works:
> 
> * Remuxing
> * Decoding (both original and remuxed file)
> 
> Issues:
> 
> * Need -copyinkf when remuxing. Demuxer problably needs a strategic
> patch

I finally got around to fixing this. It happens due to
compute_pkt_fields() not setting AV_PKT_FLAG_KEY. The simplest and most
intuitive solution to this problem IMO is to consider subtitles to be
intra-only. I am not aware of any counter case, but please reply if you
know of one.

The closest thing I can think of is actually dvbsub. It seems to use
separate packets for displaying and clearing the rects. Accidentally
getting a "clear" packet might not be much of a problem though.

Patch attached. Passes regtests. Together with the patch I posted above
dvbsub copying works.

/Tomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: intra_only_subs.patch
Type: text/x-patch
Size: 548 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101019/f1c5c50a/attachment.bin>
-------------- 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/20101019/f1c5c50a/attachment.pgp>