[FFmpeg-devel] [PATCH 5/5] avformat/movenc: add support for AV1 streams

[Michael Niedermayer]

On Thu, Jul 12, 2018 at 10:59:44AM -0300, James Almer wrote:
> On 7/12/2018 8:26 AM, Michael Niedermayer wrote:
> > On Mon, Jul 09, 2018 at 03:26:54PM -0300, James Almer wrote:
> >> Signed-off-by: James Almer <jamrial at gmail.com>
> >> ---
> >> ff_av1_filter_obus() could eventually be replaced by an autoinserted
> >> filter_units bsf, assuming it doesn't slow down the muxing process
> >> too much (CBS is fast reading packets, but not so much assembling and
> >> writing packets).
> >> ff_isom_write_av1c() however can't be replaced given filter_units
> >> doesn't handle extradata (either codecpar or packet side data).
> > [...]
> >> +#endif /* AVFORMAT_AV1_H */
> >> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> >> index fe0a244a8f..784df6d08d 100644
> >> --- a/libavformat/movenc.c
> >> +++ b/libavformat/movenc.c
> >> @@ -30,6 +30,7 @@
> >>  #include "riff.h"
> >>  #include "avio.h"
> >>  #include "isom.h"
> >> +#include "av1.h"
> >>  #include "avc.h"
> >>  #include "libavcodec/ac3_parser_internal.h"
> >>  #include "libavcodec/dnxhddata.h"
> >> @@ -1163,6 +1164,19 @@ static int mov_write_d263_tag(AVIOContext *pb)
> >>      return 0xf;
> >>  }
> >>  
> >> +static int mov_write_av1c_tag(AVIOContext *pb, MOVTrack *track)
> >> +{
> >> +    int64_t pos = avio_tell(pb);
> >> +
> >> +    avio_wb32(pb, 0);
> >> +    ffio_wfourcc(pb, "av1C");
> >> +    avio_w8(pb, 0); /* version */
> >> +    avio_wb24(pb, 0); /* flags */
> >> +    avio_w8(pb, 0); /* reserved (3), initial_presentation_delay_present (1), initial_presentation_delay_minus_one/reserved (4) */
> >> +    ff_isom_write_av1c(pb, track->vos_data, track->vos_len);
> >> +    return update_size(pb, pos);
> >> +}
> >> +
> >>  static int mov_write_avcc_tag(AVIOContext *pb, MOVTrack *track)
> >>  {
> >>      int64_t pos = avio_tell(pb);
> >> @@ -2009,6 +2023,8 @@ static int mov_write_video_tag(AVIOContext *pb, MOVMuxContext *mov, MOVTrack *tr
> >>              mov_write_uuid_tag_ipod(pb);
> >>      } else if (track->par->codec_id == AV_CODEC_ID_VP9) {
> >>          mov_write_vpcc_tag(mov->fc, pb, track);
> >> +    } else if (track->par->codec_id == AV_CODEC_ID_AV1) {
> >> +        mov_write_av1c_tag(pb, track);
> >>      } else if (track->par->codec_id == AV_CODEC_ID_VC1 && track->vos_len > 0)
> >>          mov_write_dvc1_tag(pb, track);
> >>      else if (track->par->codec_id == AV_CODEC_ID_VP6F ||
> >> @@ -5319,6 +5335,13 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt)
> >>          } else {
> >>              size = ff_hevc_annexb2mp4(pb, pkt->data, pkt->size, 0, NULL);
> >>          }
> >> +    } else if (par->codec_id == AV_CODEC_ID_AV1) {
> >> +        if (trk->hint_track >= 0 && trk->hint_track < mov->nb_streams) {
> >> +            ff_av1_filter_obus_buf(pkt->data, &reformatted_data, &size);
> >> +            avio_write(pb, reformatted_data, size);
> >> +        } else {
> >> +            size = ff_av1_filter_obus(pb, pkt->data, pkt->size);
> >> +        }
> >>  #if CONFIG_AC3_PARSER
> >>      } else if (par->codec_id == AV_CODEC_ID_EAC3) {
> >>          size = handle_eac3(mov, pkt, trk);
> > 
> >> @@ -5438,7 +5461,7 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt)
> >>          av_log(s, AV_LOG_WARNING, "pts has no value\n");
> >>          pkt->pts = pkt->dts;
> >>      }
> >> -    if (pkt->dts != pkt->pts)
> >> +    if (pkt->dts != pkt->pts && par->codec_id != AV_CODEC_ID_AV1)
> > 
> > When is dts != pts && par->codec_id == AV_CODEC_ID_AV1 ?
> > 
> > Iam asking because if it never is then this check is not needed
> > and if it is sometimes true then i suspect the pts values will be lost
> > that is the demuxer input would differ from the muxer output, which doesnt
> > seem right
> > Maybe iam missing something
> 
> It should never be. I just added this check for extra paranoid
> precaution since the spec forbids the ctts box. I can remove it.

I dont think this is the ideal behavior then

some random thoughts:

For example if pts!= dts, ommiting the ctts complies to that one rule
but why did they mismatch and is teh result actually a well playable
file.
The user also wont know about any of this happening as there will be
no warning i think. 
And if pts != dts, which should be use dts or pts or neither ?
also iam not sure if the muxer is the best place for this.





> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I know you won't believe me, but the highest form of Human Excellence is
to question oneself and others. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180713/290cc4f1/attachment.sig>