[FFmpeg-devel] [PATCH v2] movenc: Fix conversion of the first frame for extradata-less H264/HEVC

[James Almer]

On 5/21/2020 9:15 AM, Martin Storsjö wrote:
> Move the copying of the frame to vos_data further up in the function,
> so that when writing the actual frame data for the first frame, it's
> clear that the stream really is in annex b format, for the cases where
> we create extradata from the first frame.
> 
> Alternatively - we could invert the checks for bitstream format. If
> extradata is missing, we can't pretend that the bitstream is in
> mp4 form, because we can't even know the NAL unit length prefix size
> in that case.
> 
> Also avoid creating extradata for AVC intra. If the track tag is
> an AVC intra tag, don't copy the frame into vos_data - this matches
> other existing cases of how vos_data and TAG_IS_AVCI interact in
> other places.
> ---
>  libavformat/movenc.c | 37 +++++++++++++++++++------------------
>  1 file changed, 19 insertions(+), 18 deletions(-)
> 
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index 4560064add..2e164a106f 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -5504,6 +5504,25 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt)
>          memset(trk->vos_data + trk->vos_len, 0, AV_INPUT_BUFFER_PADDING_SIZE);
>      }
>  
> +    if ((par->codec_id == AV_CODEC_ID_DNXHD ||
> +         par->codec_id == AV_CODEC_ID_H264 ||
> +         par->codec_id == AV_CODEC_ID_HEVC ||
> +         par->codec_id == AV_CODEC_ID_TRUEHD ||
> +         par->codec_id == AV_CODEC_ID_AC3 ||
> +         par->codec_id == AV_CODEC_ID_H264 ||
> +         par->codec_id == AV_CODEC_ID_HEVC) && !trk->vos_len &&
> +         !TAG_IS_AVCI(trk->tag)) {
> +        /* copy frame to create needed atoms */
> +        trk->vos_len  = size;
> +        trk->vos_data = av_malloc(size + AV_INPUT_BUFFER_PADDING_SIZE);
> +        if (!trk->vos_data) {
> +            ret = AVERROR(ENOMEM);
> +            goto err;
> +        }
> +        memcpy(trk->vos_data, pkt->data, size);
> +        memset(trk->vos_data + size, 0, AV_INPUT_BUFFER_PADDING_SIZE);
> +    }
> +
>      if (par->codec_id == AV_CODEC_ID_AAC && pkt->size > 2 &&
>          (AV_RB16(pkt->data) & 0xfff0) == 0xfff0) {
>          if (!s->streams[pkt->stream_index]->nb_frames) {
> @@ -5582,24 +5601,6 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt)
>          }
>      }
>  
> -    if ((par->codec_id == AV_CODEC_ID_DNXHD ||
> -         par->codec_id == AV_CODEC_ID_H264 ||
> -         par->codec_id == AV_CODEC_ID_HEVC ||
> -         par->codec_id == AV_CODEC_ID_TRUEHD ||
> -         par->codec_id == AV_CODEC_ID_AC3 ||
> -         par->codec_id == AV_CODEC_ID_H264 ||
> -         par->codec_id == AV_CODEC_ID_HEVC) && !trk->vos_len) {

This is not what's currently in the tree. Looks like you have a dirty
local tree from when you first added the h264 and hevc cases here.

> -        /* copy frame to create needed atoms */
> -        trk->vos_len  = size;
> -        trk->vos_data = av_malloc(size + AV_INPUT_BUFFER_PADDING_SIZE);
> -        if (!trk->vos_data) {
> -            ret = AVERROR(ENOMEM);
> -            goto err;
> -        }
> -        memcpy(trk->vos_data, pkt->data, size);
> -        memset(trk->vos_data + size, 0, AV_INPUT_BUFFER_PADDING_SIZE);
> -    }
> -
>      if (trk->entry >= trk->cluster_capacity) {
>          unsigned new_capacity = trk->entry + MOV_INDEX_CLUSTER_SIZE;
>          if (av_reallocp_array(&trk->cluster, new_capacity,

LGTM with the above fixed.