Quoting Mark Thompson (2017-01-10 00:51:10)
> From: "Ronald S. Bultje" <[email protected]>
> 
> From ffmpeg commit 2e6636aa87303d37b112e79f093ca39500f92364.
> ---
> Exactly as in the other tine (except licence header).  It might be nicer to 
> call it vp9_superframe_merge, but maintaining identical behaviour probably 
> trumps that?
> 
> 
>  libavcodec/Makefile             |   1 +
>  libavcodec/bitstream_filters.c  |   1 +
>  libavcodec/vp9_superframe_bsf.c | 205 
> ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 207 insertions(+)
>  create mode 100644 libavcodec/vp9_superframe_bsf.c
> 

Some docs would be really nice. And a changelog entry.

> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index b3cee1d0c..7be9c2880 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -769,6 +769,7 @@ OBJS-$(CONFIG_NOISE_BSF)                  += noise_bsf.o
>  OBJS-$(CONFIG_NULL_BSF)                   += null_bsf.o
>  OBJS-$(CONFIG_REMOVE_EXTRADATA_BSF)       += remove_extradata_bsf.o
>  OBJS-$(CONFIG_TEXT2MOVSUB_BSF)            += movsub_bsf.o
> +OBJS-$(CONFIG_VP9_SUPERFRAME_BSF)         += vp9_superframe_bsf.o
>  OBJS-$(CONFIG_VP9_SUPERFRAME_SPLIT_BSF)   += vp9_superframe_split_bsf.o
>  
>  # thread libraries
> diff --git a/libavcodec/bitstream_filters.c b/libavcodec/bitstream_filters.c
> index 1cea6d77a..d46fdad81 100644
> --- a/libavcodec/bitstream_filters.c
> +++ b/libavcodec/bitstream_filters.c
> @@ -38,6 +38,7 @@ extern const AVBitStreamFilter ff_null_bsf;
>  extern const AVBitStreamFilter ff_text2movsub_bsf;
>  extern const AVBitStreamFilter ff_noise_bsf;
>  extern const AVBitStreamFilter ff_remove_extradata_bsf;
> +extern const AVBitStreamFilter ff_vp9_superframe_bsf;
>  extern const AVBitStreamFilter ff_vp9_superframe_split_bsf;
>  
>  #include "libavcodec/bsf_list.c"
> diff --git a/libavcodec/vp9_superframe_bsf.c b/libavcodec/vp9_superframe_bsf.c
> new file mode 100644
> index 000000000..27445176b
> --- /dev/null
> +++ b/libavcodec/vp9_superframe_bsf.c
> @@ -0,0 +1,205 @@
> +/*
> + * Vp9 invisible (alt-ref) frame to superframe merge bitstream filter
> + * Copyright (c) 2016 Ronald S. Bultje <[email protected]>
> + *
> + * This file is part of Libav.
> + *
> + * Libav is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * Libav is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with Libav; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
> USA
> + */
> +
> +#include "libavutil/avassert.h"
> +#include "avcodec.h"
> +#include "bsf.h"
> +#include "get_bits.h"
> +
> +#define MAX_CACHE 8
> +typedef struct VP9BSFContext {
> +    int n_cache;
> +    struct CachedBuf {
> +        uint8_t *data;
> +        int size;
> +    } cache[MAX_CACHE];
> +} VP9BSFContext;
> +
> +static void stats(const struct CachedBuf *in, int n_in,
> +                  unsigned *_max, unsigned *_sum)
> +{
> +    int n;
> +    unsigned max = 0, sum = 0;
> +
> +    for (n = 0; n < n_in; n++) {
> +        unsigned sz = in[n].size;
> +
> +        if (sz > max)
> +            max = sz;
> +        sum += sz;
> +    }
> +
> +    *_max = max;
> +    *_sum = sum;
> +}
> +
> +static int merge_superframe(const struct CachedBuf *in, int n_in, AVPacket 
> *out)
> +{
> +    unsigned max, sum, mag, marker, n, sz;
> +    uint8_t *ptr;
> +    int res;
> +
> +    stats(in, n_in, &max, &sum);
> +    mag = av_log2(max) >> 3;
> +    marker = 0xC0 + (mag << 3) + (n_in - 1);
> +    sz = sum + 2 + (mag + 1) * n_in;
> +    res = av_new_packet(out, sz);
> +    if (res < 0)
> +        return res;
> +    ptr = out->data;
> +    for (n = 0; n < n_in; n++) {
> +        memcpy(ptr, in[n].data, in[n].size);
> +        ptr += in[n].size;
> +    }
> +
> +#define wloop(mag, wr) \
> +    for (n = 0; n < n_in; n++) { \
> +        wr; \
> +        ptr += mag + 1; \
> +    }
> +
> +    // write superframe with marker 110[mag:2][nframes:3]
> +    *ptr++ = marker;
> +    switch (mag) {
> +    case 0:
> +        wloop(mag, *ptr = in[n].size);
> +        break;
> +    case 1:
> +        wloop(mag, AV_WL16(ptr, in[n].size));
> +        break;
> +    case 2:
> +        wloop(mag, AV_WL24(ptr, in[n].size));
> +        break;
> +    case 3:
> +        wloop(mag, AV_WL32(ptr, in[n].size));
> +        break;
> +    }
> +    *ptr++ = marker;
> +    av_assert0(ptr == &out->data[out->size]);
> +
> +    return 0;
> +}
> +
> +static int vp9_superframe_filter(AVBSFContext *ctx, AVPacket *out)
> +{
> +    GetBitContext gb;

Should switch to the new bitstream reader I suppose.

> +    VP9BSFContext *s = ctx->priv_data;
> +    AVPacket *in;
> +    int res, invisible, profile, marker, uses_superframe_syntax = 0, n;
> +
> +    res = ff_bsf_get_packet(ctx, &in);
> +    if (res < 0)
> +        return res;
> +
> +    marker = in->data[in->size - 1];
> +    if ((marker & 0xe0) == 0xc0) {
> +        int nbytes = 1 + ((marker >> 3) & 0x3);
> +        int n_frames = 1 + (marker & 0x7), idx_sz = 2 + n_frames * nbytes;
> +
> +        uses_superframe_syntax = in->size >= idx_sz && in->data[in->size - 
> idx_sz] == marker;
> +    }
> +
> +    if ((res = init_get_bits8(&gb, in->data, in->size)) < 0)
> +        goto done;
> +
> +    get_bits(&gb, 2); // frame marker
> +    profile  = get_bits1(&gb);
> +    profile |= get_bits1(&gb) << 1;
> +    if (profile == 3) profile += get_bits1(&gb);
> +
> +    if (get_bits1(&gb)) {
> +        invisible = 0;
> +    } else {
> +        get_bits1(&gb); // keyframe
> +        invisible = !get_bits1(&gb);
> +    }
> +
> +    if (uses_superframe_syntax && s->n_cache > 0) {
> +        av_log(ctx, AV_LOG_ERROR,
> +               "Mixing of superframe syntax and naked VP9 frames not 
> supported");
> +        res = AVERROR_INVALIDDATA;

nit: ENOSYS would be more appropriate, AFAIU the bitstream is not
invalid in this case

> +        goto done;
> +    } else if ((!invisible || uses_superframe_syntax) && !s->n_cache) {
> +        // passthrough
> +        av_packet_move_ref(out, in);
> +        goto done;
> +    } else if (s->n_cache + 1 >= MAX_CACHE) {
> +        av_log(ctx, AV_LOG_ERROR,
> +               "Too many invisible frames");
> +        res = AVERROR_INVALIDDATA;
> +        goto done;
> +    }
> +
> +    s->cache[s->n_cache].size = in->size;

A bit premature to do this I think, on an error below we leave a stale
size in there, which is not good.

> +    if (invisible && !uses_superframe_syntax) {

The second condition is superfluous, no? If I'm reading correctly, it can
never be true here.

> +        s->cache[s->n_cache].data = av_malloc(in->size);
> +        if (!s->cache[s->n_cache].data) {
> +            res = AVERROR(ENOMEM);
> +            goto done;
> +        }
> +        memcpy(s->cache[s->n_cache++].data, in->data, in->size);

Feels a bit wasteful to do all those extra mallocs and copies, when you
can just store the input packet at no cost. It's probably even less
code.


-- 
Anton Khirnov
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to