[FFmpeg-devel] [PATCH] dvbsub fix transcoding

[Anshul Maheshwari]

On Sat, Jun 21, 2014 at 11:17 PM, Michael Niedermayer <michaelni at gmx.at>
wrote:

> On Sat, Jun 21, 2014 at 08:11:02PM +0530, Anshul Maheshwari wrote:
> >
> > > save_subtitle will be called once either in page_segment or in
> end_segment
> > according to option set
> > in single call to dvbsub_decode.
>
> page_segment and end_segment can be called multiple times
> in a  single call to dvbsub_decode. This will cause save_subtitle_set()
> to be called multiple times and will result in a nonsensical timestamp
> with this FFSWAP() code
>
> done

> the multiple calls will also cause other problems like memleaks i
> think
>
> Will wait till someone give sample for this. I hope there would be
something in spec
,some flag would be set when those function are called twice.

>
> > I thought of adding one more security check by checking prev_start is
> less
> > then pts , but it will also fail if they are called
> > when pts is up with its 33 bit and revolving back from 0.
> > So i did moved that code in dvbsub_decode
> >
> >
> > > returns a uninitialized variable in some cases
> > >
> > > initialized at both place
> >
> > >
> > > >
> > > >  static int dvbsub_decode(AVCodecContext *avctx,
> > > > @@ -1514,7 +1534,7 @@ static int dvbsub_decode(AVCodecContext *avctx,
> > > >              ctx->composition_id == -1 || ctx->ancillary_id == -1) {
> > > >              switch (segment_type) {
> > > >              case DVBSUB_PAGE_SEGMENT:
> > > > -                dvbsub_parse_page_segment(avctx, p, segment_length,
> > > sub);
> > > > +                ret = dvbsub_parse_page_segment(avctx, p,
> > > segment_length, sub);
> > > >                  got_segment |= 1;
> > > >                  break;
> > > >              case DVBSUB_REGION_SEGMENT:
> > > > @@ -1534,7 +1554,7 @@ static int dvbsub_decode(AVCodecContext *avctx,
> > > >                  dvbsub_parse_display_definition_segment(avctx, p,
> > > segment_length);
> > > >                  break;
> > > >              case DVBSUB_DISPLAY_SEGMENT:
> > > > -                *data_size = dvbsub_display_end_segment(avctx, p,
> > > segment_length, sub);
> > > > +                ret = dvbsub_display_end_segment(avctx, p,
> > > segment_length, sub);
> > > >                  got_segment |= 16;
> > > >                  break;
> > > >              default:
> > > > @@ -1543,6 +1563,8 @@ static int dvbsub_decode(AVCodecContext *avctx,
> > > >                  break;
> > > >              }
> > > >          }
> > > > +        if(ret > *data_size)
> > > > +            *data_size = ret;
> > >
> > > the code now is a bit more complex but it does not look more
> > > correct to me.
> > >
> > > can you explain why you set data_size not simply when you
> > > set the subtitle and leave it alone when you do not ?
> > >
> >
> > In dvbsub_display_end_segment function  save_display_set function is not
> at
> > all called,
>
> noone talked about save_display_set() which is under #ifdef DEBUG
>
>
>
ohh, I was saying about save_subtitle_set()

> > and i don't get to know whether subtitle is set there or not.
> > checking sub->num_rects in dvbsub_display_end_segment and setting return
> > acc to that.
> > does not look good to me.
> > If you want me to check that i will do that way.
>
> save_subtitle_set() sets AVSubtitle
> save_subtitle_set() should also set data_size when it sets up
> AVSubtitle, that is to indicate that AVSubtitle has been set
> no other code should set data_size, that is unless there is other
> code that sets AVSubtitle
>
> data_size instead could be set outside but then it should be
> immedeatly vissible that they match and are correct.
> Not like these patches that set data_size and AVSubtitle at different
> points in different functions in a loop and are wrong on closer
> inspection.
>

tried

New patch attached
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-fix-transcoding-dvbsub-to-dvbsub.patch
Type: text/x-patch
Size: 4812 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140622/6e48050b/attachment.bin>