Hi Rostislav,
It seems there is a bit miscommunication here. I dont want to create a full
struct. However, as James mentioned too, Content Light and Dynamic HDR10+
are two different SEI types. There's no
reason to merge them into a single struct within the HEVC decoder. I mean
to create the following struct rather than merging it to ContentLight:
typedef struct HEVCSEIDynamicHDRPlus{
int present;
AVBufferRef info;
} HEVCSEIDynamicHDRPlus;
--
Best,
Mohammad
On Fri, Jan 4, 2019 at 2:51 PM Rostislav Pehlivanov <atomnu...@gmail.com>
wrote:
> On Fri, 4 Jan 2019 at 21:08, James Almer <jamr...@gmail.com> wrote:
>
> > On 1/4/2019 3:53 PM, Rostislav Pehlivanov wrote:
> > > On Fri, 4 Jan 2019 at 18:12, Mohammad Izadi <moh.iz...@gmail.com>
> wrote:
> > >
> > >> You mean a pointer in HEVCSEIDynamicHDRPlus, not in
> HEVCSEIContentLight?
> > >> --
> > >> Best,
> > >> Mohammad
> > >>
> > >>
> > >> On Thu, Jan 3, 2019 at 5:08 PM Rostislav Pehlivanov <
> > atomnu...@gmail.com>
> > >> wrote:
> > >>
> > >>> On Thu, 3 Jan 2019 at 20:13, Mohammad Izadi <moh.iz...@gmail.com>
> > wrote:
> > >>>
> > >>>>
> > >>>> /**
> > >>>> @@ -94,6 +95,50 @@ typedef struct HEVCSEIMasteringDisplay {
> > >>>> uint32_t min_luminance;
> > >>>> } HEVCSEIMasteringDisplay;
> > >>>>
> > >>>> +typedef struct HEVCSEIDynamicHDRPlus{
> > >>>> + int present;
> > >>>> + uint8_t itu_t_t35_country_code;
> > >>>> + uint8_t application_version;
> > >>>> + uint8_t num_windows;
> > >>>> + struct {
> > >>>> + AVRational window_upper_left_corner_x;
> > >>>> + AVRational window_upper_left_corner_y;
> > >>>> + AVRational window_lower_right_corner_x;
> > >>>> + AVRational window_lower_right_corner_y;
> > >>>> + uint16_t center_of_ellipse_x;
> > >>>> + uint16_t center_of_ellipse_y;
> > >>>> + uint8_t rotation_angle;
> > >>>> + uint16_t semimajor_axis_internal_ellipse;
> > >>>> + uint16_t semimajor_axis_external_ellipse;
> > >>>> + uint16_t semiminor_axis_external_ellipse;
> > >>>> + uint8_t overlap_process_option;
> > >>>> + AVRational maxscl[3];
> > >>>> + AVRational average_maxrgb;
> > >>>> + uint8_t num_distribution_maxrgb_percentiles;
> > >>>> + struct {
> > >>>> + uint8_t percentage;
> > >>>> + AVRational percentile;
> > >>>> + } distribution_maxrgb[15];
> > >>>> + AVRational fraction_bright_pixels;
> > >>>> + uint8_t tone_mapping_flag;
> > >>>> + AVRational knee_point_x;
> > >>>> + AVRational knee_point_y;
> > >>>> + uint8_t num_bezier_curve_anchors;
> > >>>> + AVRational bezier_curve_anchors[15];
> > >>>> + uint8_t color_saturation_mapping_flag;
> > >>>> + AVRational color_saturation_weight;
> > >>>> + } params[3];
> > >>>> + AVRational targeted_system_display_maximum_luminance;
> > >>>> + uint8_t targeted_system_display_actual_peak_luminance_flag;
> > >>>> + uint8_t num_rows_targeted_system_display_actual_peak_luminance;
> > >>>> + uint8_t num_cols_targeted_system_display_actual_peak_luminance;
> > >>>> + AVRational
> targeted_system_display_actual_peak_luminance[25][25];
> > >>>> + uint8_t mastering_display_actual_peak_luminance_flag;
> > >>>> + uint8_t num_rows_mastering_display_actual_peak_luminance;
> > >>>> + uint8_t num_cols_mastering_display_actual_peak_luminance;
> > >>>> + AVRational mastering_display_actual_peak_luminance[25][25];
> > >>>> +} HEVCSEIDynamicHDRPlus;
> > >>>> +
> > >>>>
> > >>>
> > >>> There's no reason to create a new struct for this if all you're going
> > to
> > >> do
> > >>> is to copy it over and over to new frames.
> > >>> What you can do is this: on every SEI containing this thing just
> > >> allocate a
> > >>> AVDynamicHDRPlus struct via av_dynamic_hdr_plus_alloc, wrap it as an
> > >>> AVBufferRef via av_buffer_create, populate it, put it as a pointer in
> > >>> HEVCSEIContentLight and then on every frame just add it to the frame
> > via
> > >>> av_frame_new_side_data_from_buf. If a new SEI is received unref your
> > >>> pointer in HEVCSEIDynamicHDRPlus and repeat by allocating a new
> struct,
> > >>> wrapping it as a buffer, populating it, etc.
> > >>> I figure the only reason this wasn't done with other metadata was
> > because
> > >>> they were much smaller and because av_frame_new_side_data_from_buf
> > didn't
> > >>> exist until recently.
> > >>>
> > >>>
> > >>> + av_log(s->avctx, AV_LOG_DEBUG, ")");
> > >>>> + if (metadata->params[w].color_saturation_mapping_flag)
> {
> > >>>> + av_log(s->avctx, AV_LOG_DEBUG,
> > >>>> + " color_saturation_weight=%5.4f",
> > >>>> +
> > >>>> av_q2d(metadata->params[w].color_saturation_weight));
> > >>>> + }
> > >>>> + av_log(s->avctx, AV_LOG_DEBUG, "}\n");
> > >>>> + }
> > >>>> + av_log(s->avctx, AV_LOG_DEBUG,
> > >>>> + "} End of HDR10+ (SMPTE 2094-40)\n");
> > >>>> + }
> > >>>>
> > >>>
> > >>> Don't spam stuff like than in the debug log, especially not on every
> > >> single
> > >>> frame. Tools exist to print side data so just don't.
> > >>> _______________________________________________
> > >>> ffmpeg-devel mailing list
> > >>> ffmpeg-devel@ffmpeg.org
> > >>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > >>>
> > >> _______________________________________________
> > >> ffmpeg-devel mailing list
> > >> ffmpeg-devel@ffmpeg.org
> > >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > >
> > >
> > > No, I mean in HEVCSEIContentLight. Like I said, HEVCSEIDynamicHDRPlus
> > > shouldn't exist.
> >
> > By "HEVCSEIDynamicHDRPlus shouldn't exist" you mean it shouldn't contain
> > a copy of every bitstream field in the SEI?
> >
> > Content Light and Dynamic HDR10+ are two different SEI types. There's no
> > reason to merge them into a single struct within the HEVC decoder.
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
> I'm not asking you to merge them, its just that you kept the 10plus state
> there so it would make sense to replace that state (struct) with the
> avbufferref.
> In reailty if you think there's a better place to put the avbufferref state
> you'd attach to avframes you should put it there.
> And yes, HEVCSEIDynamicHDRPlus shouldn't exist, there's already a full
> struct in libavutil, so all you need to do is like I said, allocate it,
> populate it, store it somewhere and ref it to new frames.
> No need to create a new structure.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
