[FFmpeg-devel] [PATCH] libavfilter: pass QP table through the filter chain

[Michael Niedermayer]

On Wed, Sep 05, 2012 at 04:53:52PM +0200, Stefano Sabatini wrote:
> On date Wednesday 2012-09-05 12:46:43 +0200, Michael Niedermayer encoded:
> > Any volunteers to port the pp and spp filters from libmpcodec?
> > 
> > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> > ---
> >  libavfilter/avcodec.c  |    8 ++++++++
> >  libavfilter/avfilter.h |    2 ++
> >  libavfilter/buffer.c   |   23 ++++++++++++++++++++++-
> >  libavfilter/version.h  |    4 ++--
> >  libavfilter/video.c    |    1 +
> >  5 files changed, 35 insertions(+), 3 deletions(-)
> > 
> > diff --git a/libavfilter/avcodec.c b/libavfilter/avcodec.c
> > index f452303..51de6f5 100644
> > --- a/libavfilter/avcodec.c
> > +++ b/libavfilter/avcodec.c
> > @@ -42,6 +42,14 @@ int avfilter_copy_frame_props(AVFilterBufferRef *dst, const AVFrame *src)
> >          dst->video->top_field_first     = src->top_field_first;
> >          dst->video->key_frame           = src->key_frame;
> >          dst->video->pict_type           = src->pict_type;
> > +        av_freep(&dst->video->qp_table);
> > +        dst->video->qp_stride = 0;
> > +        if (src->qscale_table) {
> > +            int qsize = src->qstride ? src->qstride * ((src->height+15)/16) : (src->width+15)/16;
> > +            dst->video->qp_stride       = src->qstride;
> > +            dst->video->qp_table        = av_malloc(qsize);
> > +            memcpy(dst->video->qp_table, src->qscale_table, qsize);
> 
> What if malloc fails?

fixed


> 
> Also maybe we should factorize the code.

yes i wanted to look into that in a subsequent patch


> 
> > +        }
> >          break;
> >      case AVMEDIA_TYPE_AUDIO:
> >          dst->audio->sample_rate         = src->sample_rate;
> > diff --git a/libavfilter/avfilter.h b/libavfilter/avfilter.h
> > index c0575ce..5316559 100644
> > --- a/libavfilter/avfilter.h
> > +++ b/libavfilter/avfilter.h
> > @@ -131,6 +131,8 @@ typedef struct AVFilterBufferRefVideoProps {
> >      int top_field_first;        ///< field order
> >      enum AVPictureType pict_type; ///< picture type of the frame
> >      int key_frame;              ///< 1 -> keyframe, 0-> not
> > +    int qp_stride;                ///< qp_table stride
> > +    int8_t *qp_table;             ///< array of Quantization Parameters
> 
> "stride" is kinda an MPlayerism.
> 
> Some alternatives:
> qp_table | qp_linesize
> qp       | qp_linesize
> qp_data  | qp_linesize
> qp_data  | qp_data_linesize
> qp_table | qp_table_linesize

qp alone is wrong, qp_data is not adding much information beyond qp
the linesize vs stride is bikeshed, we use stride for the matching
field in AVFrame though.
Iam happy if people rename the field to what they prefer ...


[...]
> > @@ -173,7 +184,17 @@ void avfilter_copy_buffer_ref_props(AVFilterBufferRef *dst, AVFilterBufferRef *s
> >      dst->pos             = src->pos;
> >  
> >      switch (src->type) {
> > -    case AVMEDIA_TYPE_VIDEO: *dst->video = *src->video; break;
> > +    case AVMEDIA_TYPE_VIDEO: {
> > +        if(dst->video->qp_table)
> > +            av_freep(&dst->video->qp_table);
> > +        *dst->video = *src->video;
> > +        if(dst->video->qp_table) {
>               ^^^
>               src->video->qp_table?

they are the same

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is what and why we do it that matters, not just one of them.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120905/b2696d76/attachment.asc>