Re: [FFmpeg-devel] [PATCH 2/2] swscale/utils: Improve return codes of sws_setColorspaceDetails()
On Fri, Oct 22, 2021 at 10:55:25PM -0300, James Almer wrote: > On 10/22/2021 6:45 PM, Michael Niedermayer wrote: > > Signed-off-by: Michael Niedermayer > > --- > > libswscale/utils.c | 5 - > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/libswscale/utils.c b/libswscale/utils.c > > index 3752c3ec38c..632f6fd4648 100644 > > --- a/libswscale/utils.c > > +++ b/libswscale/utils.c > > @@ -995,7 +995,10 @@ int sws_setColorspaceDetails(struct SwsContext *c, > > const int inv_table[4], > >0, 1 << 16, 1 << 16); > > return 0; > > } > > -return -1; > > +//We do not support this combination currently, we need to cascade > > more contexts to compensate > > +if (memcmp(c->dstColorspaceTable, c->srcColorspaceTable, > > sizeof(int) * 4)) > > +return AVERROR_PATCHWELCOME; > > The doxy does not allow return values other than -1. That is strictly not true the doxy only says what will happen if "not supported" (that is return -1) it doesnt say what other error conditions will do nor what non error conditions will but for sake of not breaking anyones code its indeed better to leave this at -1 thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Those who are too smart to engage in politics are punished by being governed by those who are dumber. -- Plato signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/2] swscale/utils: Set all threads to the same colorspace even on failure
On Fri, Oct 22, 2021 at 10:53:07PM -0300, James Almer wrote: > On 10/22/2021 6:45 PM, Michael Niedermayer wrote: > > Fixes: ./ffplay dav.y4m -vf "scale=hd1080:threads=4" > > Found-by: Paul > > Signed-off-by: Michael Niedermayer > > --- > > libswscale/utils.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/libswscale/utils.c b/libswscale/utils.c > > index 367b0ea5015..3752c3ec38c 100644 > > --- a/libswscale/utils.c > > +++ b/libswscale/utils.c > > @@ -874,15 +874,16 @@ int sws_setColorspaceDetails(struct SwsContext *c, > > const int inv_table[4], > > int need_reinit = 0; > > if (c->nb_slice_ctx) { > > +int parent_ret = 0; > > for (int i = 0; i < c->nb_slice_ctx; i++) { > > int ret = sws_setColorspaceDetails(c->slice_ctx[i], inv_table, > > srcRange, table, dstRange, > > brightness, contrast, > > saturation); > > if (ret < 0) > > -return ret; > > +parent_ret = ret; > > } > > -return 0; > > +return parent_ret; > > } > > handle_formats(c); > > If you make this function not propagate the error value returned by > av_image_alloc() so it will effectively return either 0 or -1 as stated by > the doxy, you can change this to just do ret |= > sws_setColorspaceDetails(...) instead of adding a new variable. I think we should propagate the return of sws_setColorspaceDetails to itself ill post a patch improving the doxy thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Observe your enemies, for they first find out your faults. -- Antisthenes signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 2/2] swscale/swscale: Improve *ColorspaceDetails() doxy
Signed-off-by: Michael Niedermayer --- libswscale/swscale.h | 8 1 file changed, 8 insertions(+) diff --git a/libswscale/swscale.h b/libswscale/swscale.h index 59610d03426..daa53dc01ee 100644 --- a/libswscale/swscale.h +++ b/libswscale/swscale.h @@ -318,14 +318,22 @@ unsigned int sws_receive_slice_alignment(const struct SwsContext *c); * @param brightness 16.16 fixed point brightness correction * @param contrast 16.16 fixed point contrast correction * @param saturation 16.16 fixed point saturation correction +#if LIBSWSCALE_VERSION_MAJOR > 6 + * @return negative error code on error, non negative otherwise +#else * @return -1 if not supported +#endif */ int sws_setColorspaceDetails(struct SwsContext *c, const int inv_table[4], int srcRange, const int table[4], int dstRange, int brightness, int contrast, int saturation); /** +#if LIBSWSCALE_VERSION_MAJOR > 6 + * @return negative error code on error, non negative otherwise +#else * @return -1 if not supported +#endif */ int sws_getColorspaceDetails(struct SwsContext *c, int **inv_table, int *srcRange, int **table, int *dstRange, -- 2.17.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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 1/2] swscale/utils: Improve return codes of sws_setColorspaceDetails()
Signed-off-by: Michael Niedermayer --- libswscale/utils.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/libswscale/utils.c b/libswscale/utils.c index 3752c3ec38c..c726922527b 100644 --- a/libswscale/utils.c +++ b/libswscale/utils.c @@ -995,7 +995,10 @@ int sws_setColorspaceDetails(struct SwsContext *c, const int inv_table[4], 0, 1 << 16, 1 << 16); return 0; } -return -1; +//We do not support this combination currently, we need to cascade more contexts to compensate +if (c->cascaded_context[0] && memcmp(c->dstColorspaceTable, c->srcColorspaceTable, sizeof(int) * 4)) +return -1; //AVERROR_PATCHWELCOME; +return 0; } if (!isYUV(c->dstFormat) && !isGray(c->dstFormat)) { -- 2.17.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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avfilter/vf_guided: support enhanced guided filter
I would like to explain the supported modes in the guided filter, (1) Basic mode shows better denoising performance compared to bilateral filter and can be applied in many applications such as detail enhancement, dehazing, and so on, which is illustrated in paper http://kaiminghe.com/eccv10/. (2) Fast basic mode is the fast algorithm of basic mode by downsampling method, which is recommended by http://kaiminghe.com/eccv10/. When the downsampling ratio is too large, blocking artifacts may occur. We can further improve this next step. (3) Enhanced mode is based on the local edge information, which can better preserve the detailed information. We propose this mode in the second period of GSoC. (4) Fast enhanced mode is the fast algorithm of enhanced mode by downsampling method. And the memory reallocation is indeed an issue of this filter, which can be improved. All in all, we believe that guided filter is an important tool and has been proved to be very useful in many applications. So keep this filter is a better choice. Bests, Xuewei Paul B Mahol 于2021年10月22日周五 下午3:28写道: > > > On Wed, Oct 13, 2021 at 3:57 PM Paul B Mahol wrote: > >> Why this filter is being applied at first place at all? >> >> It is extraordinary slow and used very slow algorithm and reallocates >> memory all the time. >> > > Additionally so called fast mode, which is indeed faster than bilateral > filter, is unusable and have blocking artifacts. > > > I vote for removal of this filter. > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avfilter/vf_guided: support enhanced guided filter
On Sat, Oct 23, 2021 at 4:56 AM Xuewei Meng wrote: > I would like to explain the supported modes in the guided filter, > (1) Basic mode shows better denoising performance compared to bilateral > filter and can be applied in many applications such as detail enhancement, > dehazing, and so on, which is illustrated in paper > http://kaiminghe.com/eccv10/. > Can you back up your claims with actual number, like what parameters were used? > (2) Fast basic mode is the fast algorithm of basic mode by downsampling > method, which is recommended by http://kaiminghe.com/eccv10/. When the > downsampling ratio is too large, blocking artifacts may occur. We can > further improve this next step. > Nope, this mode is useless, it produce by default the blocking artifacts. Provide line parameters when it can be proved useful. > > (3) Enhanced mode is based on the local edge information, which can better > preserve the detailed information. We propose this mode in the second > period of GSoC. > (4) Fast enhanced mode is the fast algorithm of enhanced mode by > downsampling method. > > And the memory reallocation is indeed an issue of this filter, which can > be improved. > > All in all, we believe that guided filter is an important tool and has > been proved to be very useful in many applications. So keep this filter is > a better choice. > Citations needed. > Some results are attached. > You used default parameters for bilateral filter? That is not fair comparison. Also you omitted speed comparison. Of course bilateral filter implementation is recursive approximation and does not work for very big sigmaS and sigmaR, but that is less important. > Bests, > Xuewei > > > > Paul B Mahol 于2021年10月22日周五 下午3:28写道: > >> >> >> On Wed, Oct 13, 2021 at 3:57 PM Paul B Mahol wrote: >> >>> Why this filter is being applied at first place at all? >>> >>> It is extraordinary slow and used very slow algorithm and reallocates >>> memory all the time. >>> >> >> Additionally so called fast mode, which is indeed faster than bilateral >> filter, is unusable and have blocking artifacts. >> >> >> I vote for removal of this filter. >> > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/3] avcodec/hevcdec: Export Dolby Vision RPUs as side data
On 10/21/2021 10:48 AM, Derek Buitenhuis wrote: Signed-off-by: Derek Buitenhuis --- * The NAL reordering approach is a result of discussions with Anton and James a few months ago. * I've put the NAL reordering in ff_h2645_packet_split, rather than a bitstream filter for a few reasons: * I don't think having a decoder's behavior rely on the presence of a bitstream filter is good architecture / design. * This spliting is only usd or useful to decoding. --- libavcodec/h2645_parse.c | 28 libavcodec/hevcdec.c | 32 libavcodec/hevcdec.h | 3 +++ libavcodec/version.h | 2 +- 4 files changed, 64 insertions(+), 1 deletion(-) diff --git a/libavcodec/h2645_parse.c b/libavcodec/h2645_parse.c index 6fbe97ad4a..04348c3a73 100644 --- a/libavcodec/h2645_parse.c +++ b/libavcodec/h2645_parse.c @@ -517,6 +517,34 @@ int ff_h2645_packet_split(H2645Packet *pkt, const uint8_t *buf, int length, pkt->nb_nals++; } +/* + * Due to limitions in avcodec's current frame threading code, it cannot + * handle adding frame side data in any place except before the slice has + * started decoding. Since Dolby Vision RPUs (which appear as NAL type 62) + * are appended to the AU, this is a poblematic. As a hack around this, we + * move any RPUs to before the first slice NAL. + */ +if (codec_id == AV_CODEC_ID_HEVC && pkt->nb_nals > 1 && pkt->nals[pkt->nb_nals - 1].type == HEVC_NAL_UNSPEC62 && +pkt->nals[pkt->nb_nals - 1].data[0] == 0x7C && pkt->nals[pkt->nb_nals - 1].data[1] == 0x01) { 0x7c01 is just the NAL header that signals type 62, nuh_layer_id 0 and nuh_temporal_id 0. forbidden_zero_bit= 0 nal_unit_type = 10 nuh_layer_id = 00 nuh_temporal_id_plus1 =001 So the last two checks can be changed to !pkt->nals[pkt->nb_nals - 1].nuh_layer_id && !pkt->nals[pkt->nb_nals - 1].temporal_id + +int first_non_slice = 0; +H2645NAL *tmp = av_malloc(pkt->nb_nals * sizeof(H2645NAL)); +if (!tmp) +return AVERROR(ENOMEM); + +for (int i = pkt->nb_nals - 1; i >= 0; i--) { +if (pkt->nals[i].type < HEVC_NAL_VPS) +first_non_slice = i; +tmp[i] = pkt->nals[i]; +} + +pkt->nals[first_non_slice] = pkt->nals[pkt->nb_nals - 1]; +for (int i = first_non_slice + 1; i < pkt->nb_nals; i++) +pkt->nals[i] = tmp[i - 1]; + +av_free(tmp); This is horrible, but i guess there's no alternative without autoinserting a bsf. +} + return 0; } diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c index 246ffd7d80..5ccee2aa4e 100644 --- a/libavcodec/hevcdec.c +++ b/libavcodec/hevcdec.c @@ -2950,6 +2950,17 @@ static int set_side_data(HEVCContext *s) } } +if (s->rpu_buf) { +AVFrameSideData *rpu = av_frame_new_side_data(out, AV_FRAME_DATA_DOVI_RPU, s->rpu_buf_size); +if (!rpu) +return AVERROR(ENOMEM); + +memcpy(rpu->data, s->rpu_buf, s->rpu_buf_size); + +av_freep(&s->rpu_buf); +s->rpu_buf_size = 0; +} + return 0; } @@ -3224,6 +3235,23 @@ static int decode_nal_unit(HEVCContext *s, const H2645NAL *nal) case HEVC_NAL_AUD: case HEVC_NAL_FD_NUT: break; +case HEVC_NAL_UNSPEC62: +/* + * Check for RPU delimiter. + * + * Dolby Vision RPUs masquerade as unregistered NALs of type 62 and start with + * 0x7C01. What i said above. It's nothing Dolby specific. Any NAL type 62 could be like that. If there's no spec we could use to actually parse the bitstream after the NAL header, then we will end up propagating pretty much anything using NAL type 62 as Dolby RPU. + */ +if (nal->size > 2 && nal->data[0] == 0x7C && nal->data[1] == 0x01) { +s->rpu_buf = av_malloc(nal->raw_size - 2); You should use an AVBufferRef for this that you then attach to the frame using av_frame_new_side_data_from_buf(). And you need to keep threads in sync by replacing the dst context's rpu_buf with the src context rpu_buf with av_buffer_replace() in hevc_update_thread_context(). +if (!s->rpu_buf) +return AVERROR(ENOMEM); + +memcpy(s->rpu_buf, nal->raw_data + 2, nal->raw_size - 2); + +s->rpu_buf_size = nal->raw_size - 2; +} +break; default: av_log(s->avctx, AV_LOG_INFO, "Skipping NAL unit %d\n", s->nal_unit_type); @@ -3512,6 +3540,8 @@ static av_cold int hevc_decode_free(AVCodecContext *avctx) pic_arrays_free(s); +av_freep(&s->rpu_buf); + av_freep(&s->md5_ctx); av_freep(&s->cabac_state); @@ -3754,6 +3784,8 @@ static void hevc_decode_flush(AVCodecContext *avctx) HEVCContext *s = avctx->priv_data; ff_hevc_flush_dpb(s); ff_
[FFmpeg-devel] [PATCH] avfilter/vf_owdenoise: relicense my code
Signed-off-by: Michael Niedermayer --- libavfilter/vf_owdenoise.c | 5 + 1 file changed, 5 insertions(+) diff --git a/libavfilter/vf_owdenoise.c b/libavfilter/vf_owdenoise.c index f15baf7e084..bb99e8f33c4 100644 --- a/libavfilter/vf_owdenoise.c +++ b/libavfilter/vf_owdenoise.c @@ -19,6 +19,11 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ +// The code written by Michael Niedermayer in 70024b6b47b9eacfe01e8f92349ca9bf1ccd7d5a:libavfilter/vf_owdenoise.c +// can also be used under the LGPL due to: +// durandal_1707, if you do all the "todo" points from vf_owdenoise.c that are in that file since 2013 then sure i would be more than happy to relicense my part of it to LGPL +// michaelni: first relicense than work + /** * @todo try to change to int * @todo try lifting based implementation -- 2.17.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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] Fix typo in pixel format warning message
--- libswscale/utils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libswscale/utils.c b/libswscale/utils.c index 367b0ea..bc42673 100644 --- a/libswscale/utils.c +++ b/libswscale/utils.c @@ -1287,7 +1287,7 @@ av_cold int sws_init_context(SwsContext *c, SwsFilter *srcFilter, c->dstRange |= handle_jpeg(&c->dstFormat); if(srcFormat!=c->srcFormat || dstFormat!=c->dstFormat) -av_log(c, AV_LOG_WARNING, "deprecated pixel format used, make sure you did set range correctly\n"); +av_log(c, AV_LOG_WARNING, "deprecated pixel format used, make sure you set range correctly\n"); if (!c->contrast && !c->saturation && !c->dstFormatBpp) sws_setColorspaceDetails(c, ff_yuv2rgb_coeffs[SWS_CS_DEFAULT], c->srcRange, -- 2.28.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] swscale/utils: Fix typo in pixel format warning message
--- libswscale/utils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libswscale/utils.c b/libswscale/utils.c index 367b0ea..bc42673 100644 --- a/libswscale/utils.c +++ b/libswscale/utils.c @@ -1287,7 +1287,7 @@ av_cold int sws_init_context(SwsContext *c, SwsFilter *srcFilter, c->dstRange |= handle_jpeg(&c->dstFormat); if(srcFormat!=c->srcFormat || dstFormat!=c->dstFormat) -av_log(c, AV_LOG_WARNING, "deprecated pixel format used, make sure you did set range correctly\n"); +av_log(c, AV_LOG_WARNING, "deprecated pixel format used, make sure you set range correctly\n"); if (!c->contrast && !c->saturation && !c->dstFormatBpp) sws_setColorspaceDetails(c, ff_yuv2rgb_coeffs[SWS_CS_DEFAULT], c->srcRange, -- 2.28.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/3] avutil: Add Dolby Vision RPU side data type
On 10/22/2021 11:02 PM, Jan Ekström wrote: > DoVi Profile 5 and 7 rendering requires the data within these RPU > entities, AFAIK (as it contains various values to correctly configure > the custom color space). So software such as libplacebo where haasn > has made efforts towards implementing the non-standard color space > utilized would benefit from having the contents. > > So while there clearly are two separate use cases, I would note that > saying "nothing would see any use from the bytes within these > structures" is maybe a bit disingenuous. Do any of thse actually work or is it just gusswork based on reverse engineering and patents (0% chance I am readng any patent or IPR). We could rename this or DOVI_PROFILE8 or RPU_BUFFER or something if you prefer. > The two use cases being: > 1. Just re-encoding the content. The contents of these RPU buffers > being unrelated and only passthrough is necessary. Yes, supported today by e.g. x265, which I imagine is the most common usecase. > 2. Trying to gather the information of these metadata units regarding > interpretation of the received image. To what end is a bit iffy - nothing can make much use of 99% of it. > Both are clearly valid use cases, and we should IMHO support both, at > least on the AVFrame level. Not that I mean that you should implement > the parsing of these metadata units, since it is clear you want to > keep away from that part of the work :) . Indeed, I was hoping this patchset would not be bogged down with "yeah implement this undocumented proprietary parser from RE or patent instead" which I obviously cannot do (dayjob, etc.) - and I think the value is ... eh ... at best. I'm not particularily fond of the idea of indefinitely killing this functionality while waiting for someone to take up the parsing. >> - and it is a legal minefield >> which I am not going to touch.) > > While with this I agree with completely. Or at least given the D > company's track record :P . That said, just parsing the metadata units > is probably much less of a swamp than actually trying to utilize the > metadata to properly render Profile 5 or 7 video. Which is why parsing it is iffy value given the risk. - Derek ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/3] avcodec/hevcdec: Export Dolby Vision RPUs as side data
On 10/23/2021 3:18 PM, James Almer wrote: > 0x7c01 is just the NAL header that signals type 62, nuh_layer_id 0 and > nuh_temporal_id 0. > > forbidden_zero_bit= 0 > nal_unit_type = 10 > nuh_layer_id = 00 > nuh_temporal_id_plus1 =001 > > So the last two checks can be changed to !pkt->nals[pkt->nb_nals - > 1].nuh_layer_id && !pkt->nals[pkt->nb_nals - 1].temporal_id Do'h, will fix. > This is horrible, but i guess there's no alternative without > autoinserting a bsf. Yeah, I couldn't think of one - and the BSF option seemed even worse. > What i said above. It's nothing Dolby specific. Any NAL type 62 could be > like that. If there's no spec we could use to actually parse the > bitstream after the NAL header, then we will end up propagating pretty > much anything using NAL type 62 as Dolby RPU. Indeed, there is no spec. As far as I know, nothing else uses UNSPEC62, though, and this is consistent with what e.g. Dolby's open source muxer and mkvtoolnix do. What do you suggest? Rename the side data to _UNSPEC62 maybe and leave it to the caller to handle it? Or, what we could do is check for the existence of stream side data containing the DOVI configuration record, which is needed to use these RPUs, I think. >> + */ >> +if (nal->size > 2 && nal->data[0] == 0x7C && nal->data[1] == 0x01) { >> +s->rpu_buf = av_malloc(nal->raw_size - 2); > > You should use an AVBufferRef for this that you then attach to the frame > using av_frame_new_side_data_from_buf(). Will do. > And you need to keep threads in sync by replacing the dst context's > rpu_buf with the src context rpu_buf with av_buffer_replace() in > hevc_update_thread_context(). Will do. Interestingly, I haven't run into any issues here with MT at all, in months of use. Luck? - Derek ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/3] avcodec/hevcdec: Export Dolby Vision RPUs as side data
On 10/23/2021 2:52 PM, Derek Buitenhuis wrote: On 10/23/2021 3:18 PM, James Almer wrote: 0x7c01 is just the NAL header that signals type 62, nuh_layer_id 0 and nuh_temporal_id 0. forbidden_zero_bit= 0 nal_unit_type = 10 nuh_layer_id = 00 nuh_temporal_id_plus1 =001 So the last two checks can be changed to !pkt->nals[pkt->nb_nals - 1].nuh_layer_id && !pkt->nals[pkt->nb_nals - 1].temporal_id Do'h, will fix. That being said, would Dolby RPU NALUs like this using other values for temporal and layer id be valid? Or can we just assume that's never happening? This is horrible, but i guess there's no alternative without autoinserting a bsf. Yeah, I couldn't think of one - and the BSF option seemed even worse. What i said above. It's nothing Dolby specific. Any NAL type 62 could be like that. If there's no spec we could use to actually parse the bitstream after the NAL header, then we will end up propagating pretty much anything using NAL type 62 as Dolby RPU. Indeed, there is no spec. As far as I know, nothing else uses UNSPEC62, though, and this is consistent with what e.g. Dolby's open source muxer and mkvtoolnix do. What do you suggest? Rename the side data to _UNSPEC62 maybe and leave it to the caller to handle it? No, i prefer having the side data be about Dolby Vision RPU. Who knows, maybe in the future Dolby realizes they had the Unregistered and even Registered User Data SEI readily available for this kind of proprietary information and start using that instead... Or, what we could do is check for the existence of stream side data containing the DOVI configuration record, which is needed to use these RPUs, I think. That sounds ideal, yes. + */ +if (nal->size > 2 && nal->data[0] == 0x7C && nal->data[1] == 0x01) { +s->rpu_buf = av_malloc(nal->raw_size - 2); You should use an AVBufferRef for this that you then attach to the frame using av_frame_new_side_data_from_buf(). Will do. And you need to keep threads in sync by replacing the dst context's rpu_buf with the src context rpu_buf with av_buffer_replace() in hevc_update_thread_context(). Will do. Interestingly, I haven't run into any issues here with MT at all, in months of use. Luck? I guess it may be because there's one such NALU per AU in this sample, so no frame depends on other threads having finished parsing/decoding their own frames and syncing the stuff stored in their thread-specific contexts. - Derek ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/3] avcodec/hevcdec: Export Dolby Vision RPUs as side data
On Sat, Oct 23, 2021 at 9:16 PM James Almer wrote: > > On 10/23/2021 2:52 PM, Derek Buitenhuis wrote: > > On 10/23/2021 3:18 PM, James Almer wrote: > >> 0x7c01 is just the NAL header that signals type 62, nuh_layer_id 0 and > >> nuh_temporal_id 0. > >> > >> forbidden_zero_bit= 0 > >> nal_unit_type = 10 > >> nuh_layer_id = 00 > >> nuh_temporal_id_plus1 =001 > >> > >> So the last two checks can be changed to !pkt->nals[pkt->nb_nals - > >> 1].nuh_layer_id && !pkt->nals[pkt->nb_nals - 1].temporal_id > > > > Do'h, will fix. > > That being said, would Dolby RPU NALUs like this using other values for > temporal and layer id be valid? Or can we just assume that's never > happening? > > > > >> This is horrible, but i guess there's no alternative without > >> autoinserting a bsf. > > > > Yeah, I couldn't think of one - and the BSF option seemed even worse. > > > >> What i said above. It's nothing Dolby specific. Any NAL type 62 could be > >> like that. If there's no spec we could use to actually parse the > >> bitstream after the NAL header, then we will end up propagating pretty > >> much anything using NAL type 62 as Dolby RPU. > > > > Indeed, there is no spec. As far as I know, nothing else uses UNSPEC62, > > though, and this is consistent with what e.g. Dolby's open source muxer > > and mkvtoolnix do. > > > > What do you suggest? Rename the side data to _UNSPEC62 maybe and leave it > > to the caller to handle it? > > No, i prefer having the side data be about Dolby Vision RPU. Who knows, > maybe in the future Dolby realizes they had the Unregistered and even > Registered User Data SEI readily available for this kind of proprietary > information and start using that instead... > They already utilize something like this for their ST.2094-10 brightness metadata (which is actually specified through ETSI etc [1]), which hilariously is also called "DoVi" in their marketing. Since effectively D has taken over NAL unit 62 for this stuff, just call it RPU_BUFFER or so, and take in everything NAL unit 62? Jan [1] https://www.etsi.org/deliver/etsi_ts/103500_103599/103572/01.03.01_60/ts_103572v010301p.pdf ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/3] avcodec/hevcdec: Export Dolby Vision RPUs as side data
On 10/23/2021 7:15 PM, James Almer wrote: >> Do'h, will fix. > > That being said, would Dolby RPU NALUs like this using other values for > temporal and layer id be valid? Or can we just assume that's never > happening? They also use 63 for an EL, according to other codebases an docs. I didn't include it in this patch, since I have no such samples or way to test. > No, i prefer having the side data be about Dolby Vision RPU. Who knows, > maybe in the future Dolby realizes they had the Unregistered and even > Registered User Data SEI readily available for this kind of proprietary > information and start using that instead... OK. >> Or, what we could do is check for the existence of stream side data >> containing >> the DOVI configuration record, which is needed to use these RPUs, I think. > > That sounds ideal, yes. Will do. > I guess it may be because there's one such NALU per AU in this sample, > so no frame depends on other threads having finished parsing/decoding > their own frames and syncing the stuff stored in their thread-specific > contexts. There must be a exactly one RPU per AU, is my understanding - anything else is malformed. I guess malformed files would run into trouble, so this should be done regardless. - Derek ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/3] avcodec/hevcdec: Export Dolby Vision RPUs as side data
On 10/23/2021 7:25 PM, Jan Ekström wrote: > Since effectively D has taken over NAL unit 62 for this stuff, just > call it RPU_BUFFER or so, and take in everything NAL unit 62? Sounds OK to me if others are OK. - Derek ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3] avcodec/libjxl: add JPEG XL decoding via libjxl
On Fri, Oct 22, 2021 at 5:38 PM Michael Niedermayer wrote: > its 2 separate libraries they are often shiped by distributions in > separate packages. libavformat can depend on libavcodec but not the > other way around. libavcodec could get upgraded without libavformat > separate patches make it more natural to keep this clean and working > if a patch chages both its a hint something might be intermingled I see, I'll split it into a libavcodec patch that handles the codec only, and then a libavformat patch for the muxers/demuxers. >Is there some public format spec of this ? I will have to reverse-engineer the libjxl code, since the spec is copyrighted and I can't access it. It might be available for a number of CHF but I can't afford it. Speaking of the bitstream format, this is starting to get a lot of code in the prober, mostly because of how permissive JXL codestreams are. I'm thinking that it might make sense to write a parser for Jpeg XL in libavcodec, and then have libavformat's jpegxl prober just call some of that same code. Thoughts? -Leo Izen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".