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

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(-)

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.