Quoting Mark Thompson (2017-05-14 23:24:11)
> ---
> libavcodec/cbs.c | 1 +
> libavcodec/cbs_h264.h | 396 +++++++++++++++
> libavcodec/cbs_h2645.c | 734 ++++++++++++++++++++++++++++
> libavcodec/cbs_h264_syntax.c | 1099
> ++++++++++++++++++++++++++++++++++++++++++
> libavcodec/cbs_internal.h | 3 +
> 5 files changed, 2233 insertions(+)
> create mode 100644 libavcodec/cbs_h264.h
> create mode 100644 libavcodec/cbs_h2645.c
> create mode 100644 libavcodec/cbs_h264_syntax.c
>
> diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c
> index 002c11cc6..93556eb91 100644
> --- a/libavcodec/cbs.c
> +++ b/libavcodec/cbs.c
> @@ -27,6 +27,7 @@
>
>
> static const CodedBitstreamType *cbs_type_table[] = {
> + &ff_cbs_type_h264,
> };
>
> int ff_cbs_init(CodedBitstreamContext *ctx,
> diff --git a/libavcodec/cbs_h264.h b/libavcodec/cbs_h264.h
> new file mode 100644
> index 000000000..8e8aae4da
> --- /dev/null
> +++ b/libavcodec/cbs_h264.h
> @@ -0,0 +1,396 @@
> +/*
> + * 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
> + */
> +
> +#ifndef AVCODEC_CBS_H264_H
> +#define AVCODEC_CBS_H264_H
> +
> +#include <stdint.h>
> +
> +
> +enum {
> + H264_MAX_SPS = 32,
> + H264_MAX_PPS = 256,
> +
> + H264_MAX_REFS = 31,
31? You sure?
> +
> + H264_MAX_CPB_CNT = 32,
> +
> + H264_MAX_RPLM = H264_MAX_REFS + 1,
> + H264_MAX_MMCO = H264_MAX_REFS * 2 + 3,
> +
> + H264_MAX_SEI_PAYLOADS = 64,
Is this a spec-defined limit?
> +};
Would be nice to share those with the already existing values.
(would be even nicer to share a lot of this code, but I understand
that's a lot more work)
> +static int cbs_h2645_fragment_add_nals(CodedBitstreamContext *ctx,
> + CodedBitstreamFragment *frag,
> + const H2645Packet *packet)
> +{
> + int err, i, start;
> +
> + start = frag->nb_units;
> + frag->nb_units += packet->nb_nals;
> +
> + err = av_reallocp_array(&frag->units, frag->nb_units,
> + sizeof(*frag->units));
Same as the comment for the other patch. Also, it seems rather weird and
fragile that you're messing with the parent's internals directly
rather than through a function call.
> + if (err < 0)
> + return err;
> + memset(frag->units + start, 0,
> + packet->nb_nals * sizeof(*frag->units));
> +
> + for (i = 0; i < packet->nb_nals; i++) {
> + CodedBitstreamUnit *unit = &frag->units[start + i];
> + const H2645NAL *nal = &packet->nals[i];
> +
> + unit->type = nal->type;
> +
> + unit->data = av_malloc(nal->size);
> + if (!unit->data)
> + return AVERROR(ENOMEM);
> + memcpy(unit->data, nal->data, nal->size);
> + unit->data_size = nal->size;
> + }
> +
> + return 0;
> +}
> +
> +static int cbs_h2645_split_fragment(CodedBitstreamContext *ctx,
> + CodedBitstreamFragment *frag,
> + int header)
> +{
> + enum AVCodecID codec_id = ctx->codec->codec_id;
> + H2645Packet packet = { 0 };
> + int err;
> +
> + if (frag->nb_units != 0)
> + return AVERROR(EINVAL);
nit: I'm not a huge fan of such "is our caller being sane?" checks,
since if the caller is insane then you're screwed either way and it can
just hide bugs.
> +
> + if (header && frag->data[0] && codec_id == AV_CODEC_ID_H264) {
> + // AVCC header.
> + CodedBitstreamH264Context *h264 = ctx->priv_data;
> + uint8_t *data = frag->data;
> + size_t size, start, end;
> + int i, count;
> +
> + h264->avcc = 1;
> + if (data[0] != 1) {
> + av_log(ctx, AV_LOG_ERROR, "Invalid AVCC header: "
> + "first byte %u.", *data);
> + return AVERROR_INVALIDDATA;
> + }
> +
> + h264->nal_length_size = (data[4] & 3) + 1;
> + count = data[5] & 0x1f;
Unchecked reads. I'd use the bytestream2 API.
> +
> + start = end = 6;
> + for (i = 0; i < count; i++) {
> + size = AV_RB16(data + end) + 2;
> + if (end + size > frag->data_size)
> + return AVERROR_INVALIDDATA;
> + end += size;
> + }
> + err = ff_h2645_packet_split(&packet, frag->data + start, end - start,
> + ctx, 1, 2, AV_CODEC_ID_H264);
> + if (err < 0) {
> + av_log(ctx, AV_LOG_ERROR, "Failed to split AVCC SPSs.\n");
> + return err;
> + }
> +
> + err = cbs_h2645_fragment_add_nals(ctx, frag, &packet);
> + ff_h2645_packet_uninit(&packet);
You could store the packet in the context and call uninit only once at
the end. It doesn't need to (and shouldn't) be called after every split.
> +static int cbs_h264_read_nal_unit(CodedBitstreamContext *ctx,
> + CodedBitstreamUnit *unit)
> +{
> + BitstreamContext bc;
> + int err;
> +
> + err = bitstream_init(&bc, unit->data, 8 * unit->data_size);
> + if (err < 0)
> + return err;
> +
> + switch (unit->type) {
> + case H264_NAL_SPS:
> + {
> + H264RawSPS *sps;
> +
> + sps = av_mallocz(sizeof(*sps));
> + if (!sps)
> + return AVERROR(ENOMEM);
> + err = cbs_h264_read_sps(ctx, &bc, sps);
> + if (err < 0) {
> + av_free(sps);
> + return err;
> + }
> +
> + cbs_h264_replace_sps(ctx, sps);
Should probably check the return value. Same below.
--
Anton Khirnov
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
