[FFmpeg-devel] [PATCH] Put remaining pieces of CODEC_FLAG_EMU_EDGE under FF_API_EMU_EDGE.

[Ronald S. Bultje]

Hi Andreas,

On Sat, Aug 22, 2015 at 8:53 AM, Andreas Cadhalpun <
andreas.cadhalpun at googlemail.com> wrote:

> On 17.08.2015 18:25, Ronald S. Bultje wrote:
> > The amv one probably looks suspicious, but since it's an intra-only
> > codec, I couldn't possibly imagine what it would use the edge for,
> > and the vsyncht fate result doesn't change, so it's probably OK.
> > ---
> >  ffmpeg_opt.c          | 2 ++
> >  ffplay.c              | 4 ++++
> >  libavcodec/mjpegenc.c | 2 ++
> >  libavcodec/snowenc.c  | 6 +++++-
> >  libavcodec/wmv2dec.c  | 2 ++
> >  5 files changed, 15 insertions(+), 1 deletion(-)
> >
> [...]
> > diff --git a/libavcodec/mjpegenc.c b/libavcodec/mjpegenc.c
> > index ee0b16e..f15de58 100644
> > --- a/libavcodec/mjpegenc.c
> > +++ b/libavcodec/mjpegenc.c
> > @@ -224,9 +224,11 @@ static int amv_encode_picture(AVCodecContext
> *avctx, AVPacket *pkt,
> >
> >      av_pix_fmt_get_chroma_sub_sample(avctx->pix_fmt, &chroma_h_shift,
> &chroma_v_shift);
> >
> > +#if FF_API_EMU_EDGE
> >      //CODEC_FLAG_EMU_EDGE have to be cleared
> >      if(s->avctx->flags & CODEC_FLAG_EMU_EDGE)
> >          return AVERROR(EINVAL);
> > +#endif
>
> This looks a bit strange indeed, though I don't understand what the
> original check
> is good for. I traced it to the assert in [1], but there isn't really an
> explanation.
> Anyway, as fate passes, this is probably OK.
>
> > diff --git a/libavcodec/snowenc.c b/libavcodec/snowenc.c
> > index 16c7e05..9f74c9e 100644
> > --- a/libavcodec/snowenc.c
> > +++ b/libavcodec/snowenc.c
> > @@ -1602,7 +1602,11 @@ static int encode_frame(AVCodecContext *avctx,
> AVPacket *pkt,
> >          s->lambda = 0;
> >      }//else keep previous frame's qlog until after motion estimation
> >
> > -    if (s->current_picture->data[0] &&
> !(s->avctx->flags&CODEC_FLAG_EMU_EDGE)) {
> > +    if (s->current_picture->data[0]
> > +#if FF_API_EMU_EDGE
> > +        && !(s->avctx->flags&CODEC_FLAG_EMU_EDGE)
> > +#endif
> > +        ) {
>
> APIchanges mentions that things should behave "as if the
> CODEC_FLAG_EMU_EDGE flag
> was always on". So shouldn't the whole if-block be removed with
> FF_API_EMU_EDGE?


I will look into this. However, typically encoder buffers are internally
owned/allocated, and the edge is used for motion search, so that might
change results and be unnecessary.

Ronald