Message ID | 20211204010905.2258652-5-tcChlisop0@gmail.com |
---|---|
State | New |
Headers | show |
Series | Add support for Matroska BlockAdditionMapping elements | expand |
Context | Check | Description |
---|---|---|
andriy/make_x86 | success | Make finished |
andriy/make_fate_x86 | success | Make fate finished |
andriy/make_ppc | success | Make finished |
andriy/make_fate_ppc | success | Make fate finished |
On Sat, Dec 04, 2021 at 02:09:04AM +0100, quietvoid wrote: > From: quietvoid <tcchlisop0@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@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(). > + > + avio_write(pb, buf, size); > + > return 32; /* 8 + 24 */ > } > > -- > 2.34.1 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On Fri, Dec 3, 2021 at 8:19 PM <lance.lmwang@gmail.com> wrote: > On Sat, Dec 04, 2021 at 02:09:04AM +0100, quietvoid wrote: > > From: quietvoid <tcchlisop0@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@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? 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@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > -- > Thanks, > Limin Wang > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
On Fri, Dec 03, 2021 at 08:27:06PM -0500, quietvoid wrote: > On Fri, Dec 3, 2021 at 8:19 PM <lance.lmwang@gmail.com> wrote: > > > On Sat, Dec 04, 2021 at 02:09:04AM +0100, quietvoid wrote: > > > From: quietvoid <tcchlisop0@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@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@ffmpeg.org > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > To unsubscribe, visit link above, or email > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > -- > > Thanks, > > Limin Wang > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On Fri, Dec 3, 2021 at 8:40 PM <lance.lmwang@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@gmail.com> wrote: > > > > > On Sat, Dec 04, 2021 at 02:09:04AM +0100, quietvoid wrote: > > > > From: quietvoid <tcchlisop0@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@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. The size addition was suggested here: https://ffmpeg.org/pipermail/ffmpeg-devel/2021-September/285877.html 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@ffmpeg.org > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > To unsubscribe, visit link above, or email > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > > -- > > > Thanks, > > > Limin Wang > > > _______________________________________________ > > > ffmpeg-devel mailing list > > > ffmpeg-devel@ffmpeg.org > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > To unsubscribe, visit link above, or email > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > -- > Thanks, > Limin Wang > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
On Fri, Dec 3, 2021 at 8:51 PM quietvoid <tcchlisop0@gmail.com> wrote: > > On Fri, Dec 3, 2021 at 8:40 PM <lance.lmwang@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@gmail.com> wrote: >> > >> > > On Sat, Dec 04, 2021 at 02:09:04AM +0100, quietvoid wrote: >> > > > From: quietvoid <tcchlisop0@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@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 > > 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@ffmpeg.org >> > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> > > > >> > > > To unsubscribe, visit link above, or email >> > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >> > > >> > > -- >> > > Thanks, >> > > Limin Wang >> > > _______________________________________________ >> > > ffmpeg-devel mailing list >> > > ffmpeg-devel@ffmpeg.org >> > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> > > >> > > To unsubscribe, visit link above, or email >> > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >> > > >> > _______________________________________________ >> > ffmpeg-devel mailing list >> > ffmpeg-devel@ffmpeg.org >> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> > >> > To unsubscribe, visit link above, or email >> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >> >> -- >> Thanks, >> Limin Wang >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >> >
On Fri, Dec 03, 2021 at 08:55:08PM -0500, quietvoid wrote: > On Fri, Dec 3, 2021 at 8:51 PM quietvoid <tcchlisop0@gmail.com> wrote: > > > > > On Fri, Dec 3, 2021 at 8:40 PM <lance.lmwang@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@gmail.com> wrote: > >> > > >> > > On Sat, Dec 04, 2021 at 02:09:04AM +0100, quietvoid wrote: > >> > > > From: quietvoid <tcchlisop0@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@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@ffmpeg.org > >> > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >> > > > > >> > > > To unsubscribe, visit link above, or email > >> > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > >> > > > >> > > -- > >> > > Thanks, > >> > > Limin Wang > >> > > _______________________________________________ > >> > > ffmpeg-devel mailing list > >> > > ffmpeg-devel@ffmpeg.org > >> > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >> > > > >> > > To unsubscribe, visit link above, or email > >> > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > >> > > > >> > _______________________________________________ > >> > ffmpeg-devel mailing list > >> > ffmpeg-devel@ffmpeg.org > >> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >> > > >> > To unsubscribe, visit link above, or email > >> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > >> > >> -- > >> Thanks, > >> Limin Wang > >> _______________________________________________ > >> ffmpeg-devel mailing list > >> ffmpeg-devel@ffmpeg.org > >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >> > >> To unsubscribe, visit link above, or email > >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > >> > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
quietvoid: > On Fri, Dec 3, 2021 at 8:40 PM <lance.lmwang@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@gmail.com> wrote: >>> >>>> On Sat, Dec 04, 2021 at 02:09:04AM +0100, quietvoid wrote: >>>>> From: quietvoid <tcchlisop0@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@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. This function is libavformat-only; there are no external users. > The size addition was suggested here: > https://ffmpeg.org/pipermail/ffmpeg-devel/2021-September/285877.html I actually suggested this to you so that you can remove the check from the function. > > 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
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); + + avio_write(pb, buf, size); + return 32; /* 8 + 24 */ }