Re: [FFmpeg-devel] [PATCH 2/2] swscale/utils: Improve return codes of sws_setColorspaceDetails()

2021-10-23 Thread Michael Niedermayer
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

2021-10-23 Thread Michael Niedermayer
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

2021-10-23 Thread Michael Niedermayer
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()

2021-10-23 Thread Michael Niedermayer
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

2021-10-23 Thread Xuewei Meng
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

2021-10-23 Thread Paul B Mahol
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

2021-10-23 Thread James Almer

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

2021-10-23 Thread Michael Niedermayer
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

2021-10-23 Thread Zack Bloom
---
 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

2021-10-23 Thread Zack Bloom
---
 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

2021-10-23 Thread Derek Buitenhuis
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

2021-10-23 Thread Derek Buitenhuis
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

2021-10-23 Thread James Almer

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

2021-10-23 Thread Jan Ekström
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

2021-10-23 Thread Derek Buitenhuis
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

2021-10-23 Thread Derek Buitenhuis
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

2021-10-23 Thread Leo Izen
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".