[FFmpeg-devel] [PATCH v8 4/5] avformat/movenc: Refactor mov_write_dvcc_dvvc_tag to use ff_isom_put_dvcc_dvvc

[lance.lmwang at gmail.com]

On Fri, Dec 03, 2021 at 08:27:06PM -0500, quietvoid wrote:
> On Fri, Dec 3, 2021 at 8:19 PM <lance.lmwang at gmail.com> wrote:
> 
> > On Sat, Dec 04, 2021 at 02:09:04AM +0100, quietvoid wrote:
> > > From: quietvoid <tcchlisop0 at gmail.com>
> > >
> > > Improves code legibility by not using bit shifts.
> > > Also avoids duplicating the dvcC/dvvC ISOM box writing code.
> > >
> > > Signed-off-by: quietvoid <tcChlisop0 at gmail.com>
> > > ---
> > >  libavformat/movenc.c | 26 +++++++++-----------------
> > >  1 file changed, 9 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> > > index 38ff90833a..79f7d70747 100644
> > > --- a/libavformat/movenc.c
> > > +++ b/libavformat/movenc.c
> > > @@ -27,6 +27,7 @@
> > >  #include "movenc.h"
> > >  #include "avformat.h"
> > >  #include "avio_internal.h"
> > > +#include "dovi_isom.h"
> > >  #include "riff.h"
> > >  #include "avio.h"
> > >  #include "isom.h"
> > > @@ -1911,6 +1912,9 @@ static int mov_write_sv3d_tag(AVFormatContext *s,
> > AVIOContext *pb, AVSphericalMa
> > >
> > >  static int mov_write_dvcc_dvvc_tag(AVFormatContext *s, AVIOContext *pb,
> > AVDOVIDecoderConfigurationRecord *dovi)
> > >  {
> > > +    uint8_t buf[ISOM_DVCC_DVVC_SIZE];
> > > +    int size;
> > > +
> > >      avio_wb32(pb, 32); /* size = 8 + 24 */
> > >      if (dovi->dv_profile > 10)
> > >          ffio_wfourcc(pb, "dvwC");
> > > @@ -1918,23 +1922,11 @@ static int
> > mov_write_dvcc_dvvc_tag(AVFormatContext *s, AVIOContext *pb, AVDOVIDe
> > >          ffio_wfourcc(pb, "dvvC");
> > >      else
> > >          ffio_wfourcc(pb, "dvcC");
> > > -    avio_w8(pb, dovi->dv_version_major);
> > > -    avio_w8(pb, dovi->dv_version_minor);
> > > -    avio_wb16(pb, (dovi->dv_profile << 9) | (dovi->dv_level << 3) |
> > > -              (dovi->rpu_present_flag << 2) | (dovi->el_present_flag <<
> > 1) |
> > > -              dovi->bl_present_flag);
> > > -    avio_wb32(pb, (dovi->dv_bl_signal_compatibility_id << 28) | 0);
> > > -
> > > -    ffio_fill(pb, 0, 4 * 4); /* reserved */
> > > -    av_log(s, AV_LOG_DEBUG, "DOVI in %s box, version: %d.%d, profile:
> > %d, level: %d, "
> > > -           "rpu flag: %d, el flag: %d, bl flag: %d, compatibility id:
> > %d\n",
> > > -           dovi->dv_profile > 10 ? "dvwC" : (dovi->dv_profile > 7 ?
> > "dvvC" : "dvcC"),
> > > -           dovi->dv_version_major, dovi->dv_version_minor,
> > > -           dovi->dv_profile, dovi->dv_level,
> > > -           dovi->rpu_present_flag,
> > > -           dovi->el_present_flag,
> > > -           dovi->bl_present_flag,
> > > -           dovi->dv_bl_signal_compatibility_id);
> > > +
> > > +    size = ff_isom_put_dvcc_dvvc(s, buf, sizeof(buf), dovi);
> >
> > I think you need check the return of ff_isom_put_dvcc_dvvc().
> >
> >
> Wouldn't it necessarily return the constant ISOM_DVCC_DVVC_SIZE in this
> case?

yes, now it'll return negative error code in one error case. Why the out parameter is
using array instead of pointer, as the size of buffer is one of parameters also.
So if it's array, why it's necessary to check the size?

> Since the only error condition is if the passed size is lower than that,
> but the buffer is constant sized.
> 
> And the PutBitContext, being flushed, would return the same value.
> 
> 
> > > +
> > > +    avio_write(pb, buf, size);
> > > +
> > >      return 32; /* 8 + 24 */
> > >  }
> > >
> > > --
> > > 2.34.1
> > >
> > > _______________________________________________
> > > ffmpeg-devel mailing list
> > > ffmpeg-devel at ffmpeg.org
> > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > >
> > > To unsubscribe, visit link above, or email
> > > ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> >
> > --
> > Thanks,
> > Limin Wang
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> >
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".

-- 
Thanks,
Limin Wang