[FFmpeg-devel] [PATCH 2/2] libavcodec/mjpeg: remove YUVJ mentions

[Paul B Mahol]

On 12/11/17, Michael Niedermayer <michael at niedermayer.cc> wrote:
> On Mon, Dec 11, 2017 at 10:58:41PM +0100, Paul B Mahol wrote:
>> On 12/11/17, Michael Niedermayer <michael at niedermayer.cc> wrote:
>> > On Mon, Dec 11, 2017 at 12:09:33PM -0500, Vittorio Giovara wrote:
>> >> >* On 12/8/17, Paul B Mahol <onemda at gmail.com <http://gmail.com>>*
>> >> >> On 12/8/17, Vittorio Giovara <vittorio.giovara at gmail.com
>> >> >> <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel>> wrote:
>> >> >*> If we were to break this feature, I'd suggest going the full route
>> >> > of
>> >> *>*> adding a PixelFormaton and work on a sws alternative (one is
>> >> allowed
>> >> to
>> >> *>*> dream no?).
>> >> *>
>> >> > This is step to right direction, why would adding yet another API be
>> >> > better
>> >> > solution?
>> >> >
>> >> > J formats are hack - misfeature - most obvious reason why nobody
>> >> > added
>> >> > 10bit J formats, or one of alpha. Calling it otherwise, points to
>> >> > severe
>> >> > lack of understanding of problem.
>> >>
>> >> I am perfectly aware that the J formats are a hack, that's why it
>> >> would
>> >> be
>> >> nice to avoid introducing more hacks to get rid of them.
>> >> As it has been pointed out in the other thread, simply adding
>> >> .color_range
>> >> does not properly solve this problem, it is a breaking change without
>> >> proper deprecation period, and in general it seems like not a good
>> >> idea
>> >> API-wise to add an endless number of fields to AVCodec.
>> >> Like I said, having dealt with the problem in the past, the only way I
>> >> suggest to go forward is decoupling codec/filter negotiation from
>> >> pixel
>> >> formats and use a different scaling/color conversion library.
>> >
>> > The problem is completely unrelated to the scaling/color conversion
>> > library. Its difficult to remove them from libavcodec and libavfilter.
>> > And i belive paul is doing good and hard work here.
>> >
>> > If there is a issue in swscale from spliting range out, iam happy to
>> > look into fixing that if someone gives me a reproduceable test case
>> >
>> > thx
>>
>> Just create separate pixel format for every color range, bitdepth,
>> colorspace, etc combination.
>>
>> Or you want pixel formats as objects?
>>
>
>> struct AVPixelFormatHax {
>>     enum ColorMissingSpace color_space;
>>     enum ColorFillRange color_range;
>>     enum ColorPrimus  color_primaries;
>>     int  nb_components;
>>     int *bitdepths;
>>     int  nb_planes;
>>
>>     Copy big pixdesc struct and add more stuff here... (Can this be
>> done in pure C?)
>> }
>
> If you go for a struct, spliting this in 2 parts
> the first being the physical layout of the samples
> (plane numbers, depth packing variant)
> and the 2nd being the logical
> that is what the samples actually represent (colorspace, range, ...)
> might make sense
>
> struct AVPixelSoup {
>     enum AVPixelFormat pix_fmt; // This describes the physical layout
>     struct ColorWhateverDescriptor color_descriptor; // this described the
> logical content
> }
>
> struct ColorWhateverDescriptor{
>     enum ColorMissingSpace color_space;
>     enum ColorFillRange color_range;
>     enum ColorPrimus  color_primaries;
> }
>
> Some advantages of this are
> pixel_formats as they are currently largly stay (YUVJ goes,
> planar RGB might be merged with planar YUV or may be left if its more
> practical, RGB might be merged with BGR or left)
>
> because pixel formats largly stay as they are alot of code and API
> related to it should not need to be changed.
>
> Code that cares just about physical layout (copy, crop, ...) can just
> work with the pixel_formats as it always did
>
> about putting everything in a single (large) enum, i wonder how
> large that would become
> But a struct similarly could be a problem here if we have lists of all
> supported variants that litterally contain an element for each specific
> variant.
>
>
> As far as iam concerned iam really happy with any API/implementation
> that works and is practical(maintainable / has someone wanting to
> implement it / ...)
> The current code has quite a few shortcommings as it cannot
> properly list or negotiate the support for colorspaces and others.
>
> Iam not sure i should mention it or not as it might confuse but its
> related
> if one takes a step back from this and looks, the issue goes beyond pixel
> formats.
> codecs can support some pixel formats in specific profiles only
> or with specific resolutions only.
> At that level merging things in a ever growing struct is not possible
> anymore and different solutions would need to be found
> I wrote av_opt_query_ranges / AVOptionRanges to allow "introspection"
> of such information from any object that supports AVOptions.
> But this system was AFAIK largly ignored by people
> Of course its also solving a different problem than YUVJ removial here.
> But maybe it or a similar system could be usefull for more complex
> cases where a encoder or filter wants to convey to the user app what
> ranges of fields it supports together with the already set elements.

I don't see how having big struct helps here, one would still need to negotiate
color_range anyway.