Re: [FFmpeg-devel] [PATCH v2 4/5] avutil/channel_layout: add av_channel_layout_retype()
On Sun, 11 Feb 2024, James Almer wrote: On 2/4/2024 4:28 PM, Marton Balint wrote: Signed-off-by: Marton Balint --- doc/APIchanges | 3 ++ libavutil/channel_layout.c | 106 + libavutil/channel_layout.h | 40 ++ libavutil/version.h| 2 +- 4 files changed, 150 insertions(+), 1 deletion(-) diff --git a/doc/APIchanges b/doc/APIchanges index cdb9b6a458..221fea30c2 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -2,6 +2,9 @@ The last version increases of all libraries were on 2023-02-09 API changes, most recent first: +2024-02-xx - xx - lavu 58.38.100 - channel_layout.h + Add av_channel_layout_retype(). Can you please add tests for it in tests/channel_layout.c? Yes, will send a follow up series with tests once this series is merged. Regards, Marton ___ 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".
Re: [FFmpeg-devel] [PATCH v2 4/5] avutil/channel_layout: add av_channel_layout_retype()
On 2/4/2024 4:28 PM, Marton Balint wrote: Signed-off-by: Marton Balint --- doc/APIchanges | 3 ++ libavutil/channel_layout.c | 106 + libavutil/channel_layout.h | 40 ++ libavutil/version.h| 2 +- 4 files changed, 150 insertions(+), 1 deletion(-) diff --git a/doc/APIchanges b/doc/APIchanges index cdb9b6a458..221fea30c2 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -2,6 +2,9 @@ The last version increases of all libraries were on 2023-02-09 API changes, most recent first: +2024-02-xx - xx - lavu 58.38.100 - channel_layout.h + Add av_channel_layout_retype(). Can you please add tests for it in tests/channel_layout.c? ___ 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".
Re: [FFmpeg-devel] [PATCH v2 4/5] avutil/channel_layout: add av_channel_layout_retype()
On Fri, 9 Feb 2024, Anton Khirnov wrote: Quoting Marton Balint (2024-02-04 20:28:11) +/** + * Change the AVChannelOrder of a channel layout. + * + * Change of AVChannelOrder can be either lossless or lossy. In case of a + * lossless conversion all the channel designations and the associated channel + * names (if any) are kept. On a lossy conversion the channel names and channel + * designations might be lost depending on the capabilities of the desired + * AVChannelOrder. Note that some conversions are simply not possible in which + * case this function returns AVERROR(ENOSYS). + * + * The following conversions are supported: + * + * Any -> Custom : Always possible, always lossless. + * Any -> Unspecified: Always possible, lossless if channel designations + * are all unknown and channel names are not used, lossy otherwise. + * Custom-> Ambisonic : Possible if it contains ambisonic channels with + * optional non-diegetic channels in the end. Lossy if the channels have + * custom names, lossless otherwise. + * Custom-> Native : Possible if it contains native channels in native + * order. Lossy if the channels have custom names, lossless otherwise. + * + * On error this function keeps the original channel layout untouched. + * + * @param channel_layout channel layout which will be changed + * @param order the desired channel layout order + * @param flags a combination of AV_CHANNEL_LAYOUT_RETYPE_FLAG_* constants + * @return 0 if the conversion was successful and lossless or if the channel + * layout was already in the desired order + * 1 if the conversion was successful but lossy You could say 'positive number' instead of 1, which leaves us wiggle room to use other numbers in the future in a backwards compatible way. Looks ok otherwise. Ok, thanks, will apply the series with that change. Regards, Marton ___ 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".
Re: [FFmpeg-devel] [PATCH v2 4/5] avutil/channel_layout: add av_channel_layout_retype()
Quoting Marton Balint (2024-02-04 20:28:11) > +/** > + * Change the AVChannelOrder of a channel layout. > + * > + * Change of AVChannelOrder can be either lossless or lossy. In case of a > + * lossless conversion all the channel designations and the associated > channel > + * names (if any) are kept. On a lossy conversion the channel names and > channel > + * designations might be lost depending on the capabilities of the desired > + * AVChannelOrder. Note that some conversions are simply not possible in > which > + * case this function returns AVERROR(ENOSYS). > + * > + * The following conversions are supported: > + * > + * Any -> Custom : Always possible, always lossless. > + * Any -> Unspecified: Always possible, lossless if channel > designations > + * are all unknown and channel names are not used, lossy otherwise. > + * Custom-> Ambisonic : Possible if it contains ambisonic channels with > + * optional non-diegetic channels in the end. Lossy if the channels have > + * custom names, lossless otherwise. > + * Custom-> Native : Possible if it contains native channels in > native > + * order. Lossy if the channels have custom names, lossless otherwise. > + * > + * On error this function keeps the original channel layout untouched. > + * > + * @param channel_layout channel layout which will be changed > + * @param order the desired channel layout order > + * @param flags a combination of AV_CHANNEL_LAYOUT_RETYPE_FLAG_* constants > + * @return 0 if the conversion was successful and lossless or if the channel > + * layout was already in the desired order > + * 1 if the conversion was successful but lossy You could say 'positive number' instead of 1, which leaves us wiggle room to use other numbers in the future in a backwards compatible way. Looks ok otherwise. -- 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2 4/5] avutil/channel_layout: add av_channel_layout_retype()
Signed-off-by: Marton Balint
---
doc/APIchanges | 3 ++
libavutil/channel_layout.c | 106 +
libavutil/channel_layout.h | 40 ++
libavutil/version.h| 2 +-
4 files changed, 150 insertions(+), 1 deletion(-)
diff --git a/doc/APIchanges b/doc/APIchanges
index cdb9b6a458..221fea30c2 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -2,6 +2,9 @@ The last version increases of all libraries were on 2023-02-09
API changes, most recent first:
+2024-02-xx - xx - lavu 58.38.100 - channel_layout.h
+ Add av_channel_layout_retype().
+
2024-02-xx - xx - lavu 58.37.100 - channel_layout.h
Add av_channel_layout_custom_init().
diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
index 40e31e9d12..7f51c076dc 100644
--- a/libavutil/channel_layout.c
+++ b/libavutil/channel_layout.c
@@ -1036,3 +1036,109 @@ uint64_t av_channel_layout_subset(const AVChannelLayout
*channel_layout,
return ret;
}
+
+static int64_t masked_description(AVChannelLayout *channel_layout, int
start_channel)
+{
+uint64_t mask = 0;
+for (int i = start_channel; i < channel_layout->nb_channels; i++) {
+enum AVChannel ch = channel_layout->u.map[i].id;
+if (ch >= 0 && ch < 63 && mask < (1ULL << ch))
+mask |= (1ULL << ch);
+else
+return AVERROR(EINVAL);
+}
+return mask;
+}
+
+static int has_channel_names(AVChannelLayout *channel_layout)
+{
+if (channel_layout->order != AV_CHANNEL_ORDER_CUSTOM)
+return 0;
+for (int i = 0; i < channel_layout->nb_channels; i++)
+if (channel_layout->u.map[i].name[0])
+return 1;
+return 0;
+}
+
+int av_channel_layout_retype(AVChannelLayout *channel_layout, enum
AVChannelOrder order, int flags)
+{
+int allow_lossy = !(flags & AV_CHANNEL_LAYOUT_RETYPE_FLAG_LOSSLESS);
+int lossy;
+
+if (!av_channel_layout_check(channel_layout))
+return AVERROR(EINVAL);
+
+if (channel_layout->order == order)
+return 0;
+
+switch (order) {
+case AV_CHANNEL_ORDER_UNSPEC: {
+int nb_channels = channel_layout->nb_channels;
+if (channel_layout->order == AV_CHANNEL_ORDER_CUSTOM) {
+lossy = 0;
+for (int i = 0; i < nb_channels; i++) {
+if (channel_layout->u.map[i].id != AV_CHAN_UNKNOWN ||
channel_layout->u.map[i].name[0]) {
+lossy = 1;
+break;
+}
+}
+} else {
+lossy = 1;
+}
+if (!lossy || allow_lossy) {
+av_channel_layout_uninit(channel_layout);
+channel_layout->order = AV_CHANNEL_ORDER_UNSPEC;
+channel_layout->nb_channels = nb_channels;
+return lossy;
+}
+return AVERROR(ENOSYS);
+}
+case AV_CHANNEL_ORDER_NATIVE:
+if (channel_layout->order == AV_CHANNEL_ORDER_CUSTOM) {
+int64_t mask = masked_description(channel_layout, 0);
+if (mask < 0)
+return AVERROR(ENOSYS);
+lossy = has_channel_names(channel_layout);
+if (!lossy || allow_lossy) {
+av_channel_layout_uninit(channel_layout);
+av_channel_layout_from_mask(channel_layout, mask);
+return lossy;
+}
+}
+return AVERROR(ENOSYS);
+case AV_CHANNEL_ORDER_CUSTOM: {
+AVChannelLayout custom = { 0 };
+int ret = av_channel_layout_custom_init(,
channel_layout->nb_channels);
+if (ret < 0)
+return ret;
+if (channel_layout->order != AV_CHANNEL_ORDER_UNSPEC)
+for (int i = 0; i < channel_layout->nb_channels; i++)
+custom.u.map[i].id =
av_channel_layout_channel_from_index(channel_layout, i);
+av_channel_layout_uninit(channel_layout);
+*channel_layout = custom;
+return 0;
+}
+case AV_CHANNEL_ORDER_AMBISONIC:
+if (channel_layout->order == AV_CHANNEL_ORDER_CUSTOM) {
+int64_t mask;
+int nb_channels = channel_layout->nb_channels;
+int order = ambisonic_order(channel_layout);
+if (order < 0)
+return AVERROR(ENOSYS);
+mask = masked_description(channel_layout, (order + 1) * (order +
1));
+if (mask < 0)
+return AVERROR(ENOSYS);
+lossy = has_channel_names(channel_layout);
+if (!lossy || allow_lossy) {
+av_channel_layout_uninit(channel_layout);
+channel_layout->order = AV_CHANNEL_ORDER_AMBISONIC;
+channel_layout->nb_channels = nb_channels;
+channel_layout->u.mask = mask;
+return lossy;
+}
+}
+return AVERROR(ENOSYS);
+default:
+return AVERROR(EINVAL);
+}
+}
diff --git a/libavutil/channel_layout.h
Re: [FFmpeg-devel] [PATCH v2 4/5] avutil/channel_layout: add av_channel_layout_retype()
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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2 4/5] avutil/channel_layout: add av_channel_layout_retype()
On Thu, 1 Feb 2024, Anton Khirnov wrote: Quoting Marton Balint (2024-02-01 00:01:36) diff --git a/libavutil/channel_layout.h b/libavutil/channel_layout.h index 37629ab5d2..7e27a00d39 100644 --- a/libavutil/channel_layout.h +++ b/libavutil/channel_layout.h @@ -817,6 +817,17 @@ int av_channel_layout_check(const AVChannelLayout *channel_layout); */ int av_channel_layout_compare(const AVChannelLayout *chl, const AVChannelLayout *chl1); +/** + * Try changing the AVChannelOrder of a channel layout. 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... So if you have a preference, I can change this (and document it), I don't particularly feel strongly about any of the two approaches. Probably there is no bad choce as long as it is properly documented. + * + * @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. Regards, Marton ___ 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".
Re: [FFmpeg-devel] [PATCH v2 4/5] avutil/channel_layout: add av_channel_layout_retype()
Quoting Marton Balint (2024-02-01 00:01:36) > diff --git a/libavutil/channel_layout.h b/libavutil/channel_layout.h > index 37629ab5d2..7e27a00d39 100644 > --- a/libavutil/channel_layout.h > +++ b/libavutil/channel_layout.h > @@ -817,6 +817,17 @@ int av_channel_layout_check(const AVChannelLayout > *channel_layout); > */ > int av_channel_layout_compare(const AVChannelLayout *chl, const > AVChannelLayout *chl1); > > +/** > + * Try changing the AVChannelOrder of a channel layout. 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. > + * > + * @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 -- 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2 4/5] avutil/channel_layout: add av_channel_layout_retype()
v2: add conversion from custom layout to ambisonic
Signed-off-by: Marton Balint
---
doc/APIchanges | 3 ++
libavutil/channel_layout.c | 74 ++
libavutil/channel_layout.h | 11 ++
libavutil/version.h| 2 +-
4 files changed, 89 insertions(+), 1 deletion(-)
diff --git a/doc/APIchanges b/doc/APIchanges
index 8e8498f803..ce1e816fa5 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -2,6 +2,9 @@ The last version increases of all libraries were on 2023-02-09
API changes, most recent first:
+2024-02-xx - xx - lavu 58.38.100 - channel_layout.h
+ Add av_channel_layout_retype().
+
2024-02-xx - xx - lavu 58.37.100 - channel_layout.h
Add av_channel_layout_from_custom().
diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
index 0810d32bf6..056b7f7c10 100644
--- a/libavutil/channel_layout.c
+++ b/libavutil/channel_layout.c
@@ -1036,3 +1036,77 @@ uint64_t av_channel_layout_subset(const AVChannelLayout
*channel_layout,
return ret;
}
+
+static int64_t masked_description(AVChannelLayout *channel_layout, int
start_channel)
+{
+uint64_t mask = 0;
+for (int i = start_channel; i < channel_layout->nb_channels; i++) {
+enum AVChannel ch = channel_layout->u.map[i].id;
+if (ch >= 0 && ch < 63 && mask < (1ULL << ch))
+mask |= (1ULL << ch);
+else
+return AVERROR(EINVAL);
+}
+return mask;
+}
+
+int av_channel_layout_retype(AVChannelLayout *channel_layout, enum
AVChannelOrder order)
+{
+if (!av_channel_layout_check(channel_layout))
+return AVERROR(EINVAL);
+
+if (channel_layout->order == order)
+return 0;
+
+switch (order) {
+case AV_CHANNEL_ORDER_UNSPEC: {
+int nb_channels = channel_layout->nb_channels;
+av_channel_layout_uninit(channel_layout);
+channel_layout->order = AV_CHANNEL_ORDER_UNSPEC;
+channel_layout->nb_channels = nb_channels;
+return 0;
+}
+case AV_CHANNEL_ORDER_CUSTOM: {
+AVChannelLayout custom = { 0 };
+int ret = av_channel_layout_from_custom(,
channel_layout->nb_channels);
+if (ret < 0)
+return ret;
+if (channel_layout->order != AV_CHANNEL_ORDER_UNSPEC)
+for (int i = 0; i < channel_layout->nb_channels; i++)
+custom.u.map[i].id =
av_channel_layout_channel_from_index(channel_layout, i);
+av_channel_layout_uninit(channel_layout);
+*channel_layout = custom;
+return 0;
+}
+case AV_CHANNEL_ORDER_NATIVE:
+if (channel_layout->order == AV_CHANNEL_ORDER_CUSTOM) {
+int64_t mask = masked_description(channel_layout, 0);
+if (mask < 0)
+return 1;
+av_channel_layout_uninit(channel_layout);
+return av_channel_layout_from_mask(channel_layout, mask);
+} else {
+return 1;
+}
+case AV_CHANNEL_ORDER_AMBISONIC:
+if (channel_layout->order == AV_CHANNEL_ORDER_CUSTOM) {
+int64_t mask;
+int nb_channels = channel_layout->nb_channels;
+int order = ambisonic_order(channel_layout);
+if (order < 0)
+return 1;
+mask = masked_description(channel_layout, (order + 1) * (order +
1));
+if (mask < 0)
+return 1;
+av_channel_layout_uninit(channel_layout);
+channel_layout->order = AV_CHANNEL_ORDER_AMBISONIC;
+channel_layout->nb_channels = nb_channels;
+channel_layout->u.mask = mask;
+return 0;
+} else {
+return 1;
+}
+default:
+return 1;
+}
+}
diff --git a/libavutil/channel_layout.h b/libavutil/channel_layout.h
index 37629ab5d2..7e27a00d39 100644
--- a/libavutil/channel_layout.h
+++ b/libavutil/channel_layout.h
@@ -817,6 +817,17 @@ int av_channel_layout_check(const AVChannelLayout
*channel_layout);
*/
int av_channel_layout_compare(const AVChannelLayout *chl, const
AVChannelLayout *chl1);
+/**
+ * Try changing the AVChannelOrder of a channel layout.
+ *
+ * @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 code on error
+ */
+int av_channel_layout_retype(AVChannelLayout *channel_layout, enum
AVChannelOrder order);
+
/**
* @}
*/
diff --git a/libavutil/version.h b/libavutil/version.h
index 3b38f8f5da..cebf4a0acd 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,7 +79,7 @@
*/
#define LIBAVUTIL_VERSION_MAJOR 58
-#define LIBAVUTIL_VERSION_MINOR 37
+#define LIBAVUTIL_VERSION_MINOR 38
#define LIBAVUTIL_VERSION_MICRO 100
#define LIBAVUTIL_VERSION_INT
