On Tue, Nov 14, 2017 at 4:01 PM Michael Niedermayer <mich...@niedermayer.cc>
wrote:
> On Tue, Nov 14, 2017 at 10:04:02AM -0800, Aman Gupta wrote:
> > From: Aman Gupta <a...@tmm1.net>
> >
> > ---
> > libavcodec/mpeg12dec.c | 20 ++++++++++++++------
> > 1 file changed, 14 insertions(+), 6 deletions(-)
> >
> > diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
> > index 82bb1286ff..2c96dfa638 100644
> > --- a/libavcodec/mpeg12dec.c
> > +++ b/libavcodec/mpeg12dec.c
> > @@ -54,6 +54,7 @@ typedef struct Mpeg1Context {
> > int mpeg_enc_ctx_allocated; /* true if decoding context allocated */
> > int repeat_field; /* true if we must repeat the field */
> > AVPanScan pan_scan; /* some temporary storage for the
> panscan */
> > + int has_pan_scan;
> > AVStereo3D stereo3d;
> > int has_stereo3d;
> > uint8_t *a53_caption;
> > @@ -1458,6 +1459,7 @@ static void
> > mpeg_decode_sequence_display_extension(Mpeg1Context
> *s1)
> >
> > s1->pan_scan.width = 16 * w;
> > s1->pan_scan.height = 16 * h;
> > + s1->has_pan_scan = 1;
> >
> > if (s->avctx->debug & FF_DEBUG_PICT_INFO)
> > av_log(s->avctx, AV_LOG_DEBUG, "sde w:%d, h:%d\n", w, h);
> > @@ -1489,6 +1491,8 @@ static void
> > mpeg_decode_picture_display_extension(Mpeg1Context
> *s1)
> > skip_bits(&s->gb, 1); // marker
> > }
> >
> > + s1->has_pan_scan = 1;
> > +
> > if (s->avctx->debug & FF_DEBUG_PICT_INFO)
> > av_log(s->avctx, AV_LOG_DEBUG,
> > "pde (%"PRId16",%"PRId16") (%"PRId16",%"PRId16")
> (%"PRId16",%"PRId16")\n",
> > @@ -1621,12 +1625,16 @@ static int mpeg_field_start(MpegEncContext *s,
> const uint8_t *buf, int buf_size)
> > }
> > }
> >
> > - pan_scan = av_frame_new_side_data(s->current_picture_ptr->f,
> > - AV_FRAME_DATA_PANSCAN,
> > - sizeof(s1->pan_scan));
> > - if (!pan_scan)
> > - return AVERROR(ENOMEM);
> > - memcpy(pan_scan->data, &s1->pan_scan, sizeof(s1->pan_scan));
> > + if (s1->has_pan_scan) {
> > + pan_scan = av_frame_new_side_data(s->
> current_picture_ptr->f,
> > + AV_FRAME_DATA_PANSCAN,
> > + sizeof(s1->pan_scan));
> > + if (!pan_scan)
> > + return AVERROR(ENOMEM);
> > +
> > + memcpy(pan_scan->data, &s1->pan_scan, sizeof(s1->pan_scan));
> > + s1->has_pan_scan = 0;
>
> the mpeg2 spec says:
> "In the case that a given picture does not have
> a
> picture_display_extension() then the most recently decoded frame
> centre
> offset shall be used."
>
> "Following a sequence_header() the value zero shall be used for all
> frame
> centre offsets until a picture_display_extension() defines non-zero
> values."
>
> I dont think its a good idea to ommit the "most recently decoded
> frame centre" on the subset of AVFrames which do not explicitly store
> it. That would make it hard for applications which want to access this,
> an application would have to keep track of the last one as well as
> where sequence headers occured
Sorry, my mistake.
Would the patch make sense if I removed the "has_pan_scan = 0" reset? Or is
it better just to always include PANSCAN side data to make it easier for
API consumers.
Aman
>
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Old school: Use the lowest level language in which you can solve the
> problem
> conveniently.
> New school: Use the highest level language in which the latest
> supercomputer
> can solve the problem without the user falling asleep waiting.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
