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 [email protected] https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email [email protected] with subject "unsubscribe".
