On Tue, Feb 25, 2020 at 6:03 PM Mark Thompson <s...@jkqxz.net> wrote:
> On 25/02/2020 04:32, Vittorio Giovara wrote:
> > On Mon, Feb 24, 2020 at 5:18 PM Mark Thompson <s...@jkqxz.net> wrote:
> >> On 24/02/2020 21:28, Vittorio Giovara wrote:
> >>> On Sun, Feb 23, 2020 at 6:41 PM Mark Thompson <s...@jkqxz.net> wrote:
> >>>
> >>>> ---
> >>>> libavcodec/Makefile | 2 +-
> >>>> libavcodec/cbs_h265.c | 99
> +++++++++++++++++++++++++++++++++++++++++++
> >>>> libavcodec/cbs_h265.h | 18 ++++++++
> >>>> 3 files changed, 118 insertions(+), 1 deletion(-)
> >>>> create mode 100644 libavcodec/cbs_h265.c
> >>>>
> >>>> ...
> >>>> +void
> >>>>
> >>
> ff_cbs_h265_fill_sei_mastering_display(H265RawSEIMasteringDisplayColourVolume
> >>>> *mdcv,
> >>>> + const
> >>>> AVMasteringDisplayMetadata *mdm)
> >>>> +{
> >>>> + memset(mdcv, 0, sizeof(*mdcv));
> >>>> +
> >>>> + if (mdm->has_primaries) {
> >>>> + // The values in the metadata structure are fractions
> between 0
> >>>> and 1,
> >>>> + // while the SEI message contains fixed-point values with an
> >>>> increment
> >>>> + // of 0.00002. So, scale up by 50000 to convert between
> them.
> >>>> +
> >>>> + for (int a = 0; a < 3; a++) {
> >>>> + // The metadata structure stores this in RGB order, but
> the
> >>>> SEI
> >>>> + // wants it in GBR order.
> >>>> + int b = (a + 1) % 3;
> >>>>
> >>>
> >>> this is a pretty minor comment, but do you think you could use the more
> >>> legible way present in other parts of the codebase?
> >>> const int mapping[3] = {2, 0, 1};
> >>> rather than (a + 1) % 3;
> >>
> >> Ok.
> >>
> >> Is there a specific reason to make it on the stack rather than static?
> I
> >> see it's there in hevcdec.
> >>
> >
> > No particular reason, I just find it more readable, if you think it's a
> > really bad practice then you could keep the code as is.
>
> Sorry, my question wasn't very clear. I don't mind the change. But:
>
> Is there a reason why the array in hevcdec (and your suggestion) is not
> static? (Some sort of compiler optimisation effect I'm missing, maybe.)
> Intuitively it feels like it should be static const rather than being
> constructed on the stack every time the function is called.
>
Oops no, there isn't any to my knowledge, feel free to add it though.
--
Vittorio
_______________________________________________
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".
