Re: [FFmpeg-devel] [PATCH v2 4/5] avutil/channel_layout: add av_channel_layout_retype()

[Anton Khirnov <anton@khirnov.net>]

Quoting Marton Balint (2024-02-01 21:36:31)
> > What exactly is the rule for when the change succeeds or not? I would
> > expect it to be when all the channels can be represented in the new
> > order, but that is not the case for conversion to unspec.
> 
> Yes, you are right. Converting to unspec indeed makes you lose the 
> channel designations, so the conversion will not be lossless. On the other 
> hand, when you specify UNSPEC as a target, you don't actually expect to 
> keep the designations, so what is the point of returning an error...
> 
> I think this is one of those cases when both behaviour (always doing the 
> conversion, or returning a failure in case the source order is not already 
> unspec) can make sense. We have to decide though if a custom layout with 
> all channels as UNKNOWN can be losslessly converted to UNSPEC layout or 
> not. And if yes, then would not that conflict with 
> av_channel_layout_channel_from_index() which returns AV_CHAN_NONE and not 
> AV_CHAN_UNKNOWN for UNSPEC layouts...

Huh, that might actually considered a bug, returning UNKNOWN certainly
makes more sense to me.

> >
> >> + *
> >> + * @param channel_layout channel layout which will be changed
> >> + * @param order the desired channel layout order
> >> + * @return 0 on success or if the channel layout is already in the desired order
> >> + *         1 if using the desired order is not possible for the specified layout
> >
> > AVERROR(ENOSYS) seems more consistent to me
> 
> By using a positive result all negative return values can be considered 
> serious errors which have to be propagated back to the user.
> 
> In the next patch I try to simplify a custom channel layout:
> 
> +        ret = av_channel_layout_retype(ch_layout, AV_CHANNEL_ORDER_NATIVE);
> +        if (ret < 0)
> +            goto out;
> 
> I can do simply this, because I don't care if the simplification was 
> successful.

IMO policy like what errors are to be considered serious should be up to
the caller. If you asked it to get a certain order and it failed to
deliver, then I'd consider that an error state.

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".