[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
lance.lmwang at gmail.com
Sat Dec 4 04:21:04 EET 2021
On Fri, Dec 03, 2021 at 08:55:08PM -0500, quietvoid wrote:
> On Fri, Dec 3, 2021 at 8:51 PM quietvoid <tcchlisop0 at gmail.com> wrote:
>
> >
> > On Fri, Dec 3, 2021 at 8:40 PM <lance.lmwang at gmail.com> wrote:
> >
> >> 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?
> >>
> >
> > External users of the function can pass either an array or pointer.
> >
> Actually this is not true, I was looking at the wrong code.
> I don't think the size is necessary, then.
>
> The size addition was suggested here:
> > https://ffmpeg.org/pipermail/ffmpeg-devel/2021-September/285877.html
As suggested, for the array is fixed size already, so the size of array can
be removed and don't need check the size anymore.
> >
> > So in the case a pointer is used, the size is necessary.
> >
> > > 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
> >> _______________________________________________
> >> 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
More information about the ffmpeg-devel
mailing list