On Thu, Oct 14, 2021 at 9:52 AM <[email protected]> wrote: > On Thu, Oct 14, 2021 at 09:45:45AM -0400, quietvoid wrote: > > On Thu, Oct 14, 2021 at 9:36 AM <[email protected]> wrote: > > > > > On Thu, Oct 14, 2021 at 09:18:16AM -0400, f tcChlisop0 wrote: > > > > On Thu, Oct 14, 2021 at 9:10 AM <[email protected]> wrote: > > > > > > > > > From: Limin Wang <[email protected]> > > > > > > > > > > By <<Dolby Vision Streams Within the MPEG-2 Transport Stream Format > > > v1.2>> > > > > > > > > > > Signed-off-by: Limin Wang <[email protected]> > > > > > --- > > > > > libavformat/mpegts.c | 11 +++++++++-- > > > > > 1 file changed, 9 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c > > > > > index 44d9298..774964d 100644 > > > > > --- a/libavformat/mpegts.c > > > > > +++ b/libavformat/mpegts.c > > > > > @@ -2178,6 +2178,8 @@ int ff_parse_mpeg2_descriptor(AVFormatContext > > > *fc, > > > > > AVStream *st, int stream_type > > > > > AVDOVIDecoderConfigurationRecord *dovi; > > > > > size_t dovi_size; > > > > > int ret; > > > > > + int dependency_pid; > > > > > + > > > > > if (desc_end - *pp < 4) // (8 + 8 + 7 + 6 + 1 + 1 + > 1) / 8 > > > > > return AVERROR_INVALIDDATA; > > > > > > > > > > @@ -2193,7 +2195,11 @@ int > ff_parse_mpeg2_descriptor(AVFormatContext > > > *fc, > > > > > AVStream *st, int stream_type > > > > > dovi->rpu_present_flag = (buf >> 2) & 0x01; // 1 > bit > > > > > dovi->el_present_flag = (buf >> 1) & 0x01; // 1 > bit > > > > > dovi->bl_present_flag = buf & 0x01; // 1 > bit > > > > > - if (desc_end - *pp >= 20) { // 4 + 4 * 4 > > > > > + if (!dovi->bl_present_flag && desc_end - *pp >= 2) { > > > > > + buf = get16(pp, desc_end); > > > > > + dependency_pid = buf >> 3; // 13 bits > > > > > + } > > > > > + if (desc_end - *pp >= 1) { // 8 bits > > > > > buf = get8(pp, desc_end); > > > > > dovi->dv_bl_signal_compatibility_id = (buf >> 4) & > > > 0x0f; > > > > > // 4 bits > > > > > } else { > > > > > @@ -2210,12 +2216,13 @@ int > ff_parse_mpeg2_descriptor(AVFormatContext > > > *fc, > > > > > AVStream *st, int stream_type > > > > > } > > > > > > > > > > av_log(fc, AV_LOG_TRACE, "DOVI, version: %d.%d, > profile: > > > %d, > > > > > level: %d, " > > > > > - "rpu flag: %d, el flag: %d, bl flag: %d, > > > compatibility > > > > > id: %d\n", > > > > > + "rpu flag: %d, el flag: %d, bl flag: %d, > > > > > dependency_pid: %d, compatibility id: %d\n", > > > > > 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, > > > > > + dependency_pid, > > > > > dovi->dv_bl_signal_compatibility_id); > > > > > } > > > > > break; > > > > > -- > > > > > 1.8.3.1 > > > > > > > > > > _______________________________________________ > > > > > ffmpeg-devel mailing list > > > > > [email protected] > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > [email protected] with subject "unsubscribe". > > > > > > > > > > > > > Hi, this is something I had fixed in this patchset: > > > > https://ffmpeg.org/pipermail/ffmpeg-devel/2021-September/286234.html > > > > However the dependency_pid is ignored, as it has no use presently. > > > > > > > > Which patch should take precedence? > > > > > > Sorry, I have noticed your patch before. By the quick review of your > patch, > > > it's a lot function change and difficult to merge I think. I prefer to > > > fix the issue with existing code first instead of mixed function > change. > > > > > > > Okay, that makes sense. > > I will wait and rebase before resending for review then. > > > > However I'm worried my patch will still result in ignoring > dependency_pid, > > because it is not part of AVDOVIDecoderConfigurationRecord, unless it is > > added. > > I failed to find your patchset in my email archive, so I reply it here for > my comments: > - if (ret < 0) { > - av_free(dovi); > + if ((ret = ff_isom_parse_dvcc_dvvc(fc, st, *pp, desc_len, 1)) > < 0) > return ret; > - } > > I think it's wrong to use ff_isom_parse_dvcc_dvvc() here for mpegts > use DOVIVideoStreamDescriptor, ISOM use DOVIDecoderConfigurationRecord. > they're different syntax if you have checked the two specs. So your > parsing isn't > follow the specs as dependency_pid is used by DOVIVideoStreamDescriptor. >
That's true, however they only differ by dependency_pid. This is a concern I've noted here: https://ffmpeg.org/pipermail/ffmpeg-devel/2021-September/286159.html But I still went ahead with it to avoid the duplicated code. I've had no response since then, though. Either way, I'll just wait for your patches to get in. Thanks for the review. _______________________________________________ ffmpeg-devel mailing list [email protected] https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email [email protected] with subject "unsubscribe".
