Re: [FFmpeg-devel] [PATCH] lavc/bsf: add an Opus metadata bitstream filter

2020-05-05 Thread Lynne 
May 5, 2020, 11:29 by d...@lynne.ee:

> May 5, 2020, 09:59 by andreas.rheinha...@gmail.com:
>
>> Lynne:
>>
>>> The only adjustable field is the gain. Some ripping/transcoding programs 
>>> have started to use it for replaygain adjustments.
>>>
>>> Patch attached.
>>>
>>> >
>>> +typedef struct OpusBSFContext {
>>> +const AVClass *class;
>>> +int64_t gain;
>>> +} OpusBSFContext;
>>>
>> [...]
>>
>>>
>>> +static const AVOption opus_metadata_options[] = {
>>> +{ "gain", "Gain, actual amplification is pow(10, gain/(20.0*256))", 
>>> OFFSET(gain),
>>> +  AV_OPT_TYPE_INT, { .i64 = 0 }, -(INT16_MAX + 1), INT16_MAX, .flags = 
>>> FLAGS },
>>> +
>>> +{ NULL },
>>> +};
>>>
>>
>> You are using an AV_OPT_TYPE_INT parameter, yet the actual type of the
>> destination is int64_t. This won't work on big endian systems. Make gain
>> an int.
>>
>
> Thanks, made it an int, patch attached.
>
>
>
>> PS: Do we actually support two's complement architectures were
>> -(INT16_MAX + 1) != INT16_MIN? (A two's complement architecture in which
>> the representation where the sign bit is set and all other bits is unset
>> is trap representation is legal according to the C standard. Does
>> someone know whether it would also be legal according to POSIX?)
>>
>
> I think we already rely on that pretty heavily in our code.
>

Pushed.
___
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] lavc/bsf: add an Opus metadata bitstream filter

2020-05-05 Thread Lynne 
May 5, 2020, 23:15 by geo...@nsup.org:

> Lynne (12020-05-05):
>
>> Pushed.
>>
>
> I find rather rude that you did not even acknowledge the suggestion in
> this mail:
>
> https://ffmpeg.org/pipermail/ffmpeg-devel/2020-May/262151.html
>

I did acknowledge it but I didn't comment on it because I disliked the idea of 
modifying the values
that get written in the bitstream or using floats in a bsf, and I wanted to 
comment on the other
post more, so I replied to that one instead.
___
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] lavc/bsf: add an Opus metadata bitstream filter

2020-05-05 Thread Nicolas George 
Lynne (12020-05-05):
> Pushed.

I find rather rude that you did not even acknowledge the suggestion in
this mail:

https://ffmpeg.org/pipermail/ffmpeg-devel/2020-May/262151.html

-- 
  Nicolas George


signature.asc
Description: PGP signature
___
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] lavc/bsf: add an Opus metadata bitstream filter

2020-05-05 Thread Lynne 
May 5, 2020, 09:59 by andreas.rheinha...@gmail.com:

> Lynne:
>
>> The only adjustable field is the gain. Some ripping/transcoding programs 
>> have started to use it for replaygain adjustments.
>>
>> Patch attached.
>>
>> >
>> +typedef struct OpusBSFContext {
>> +const AVClass *class;
>> +int64_t gain;
>> +} OpusBSFContext;
>>
> [...]
>
>>
>> +static const AVOption opus_metadata_options[] = {
>> +{ "gain", "Gain, actual amplification is pow(10, gain/(20.0*256))", 
>> OFFSET(gain),
>> +  AV_OPT_TYPE_INT, { .i64 = 0 }, -(INT16_MAX + 1), INT16_MAX, .flags = 
>> FLAGS },
>> +
>> +{ NULL },
>> +};
>>
>
> You are using an AV_OPT_TYPE_INT parameter, yet the actual type of the
> destination is int64_t. This won't work on big endian systems. Make gain
> an int.
>

Thanks, made it an int, patch attached.



> PS: Do we actually support two's complement architectures were
> -(INT16_MAX + 1) != INT16_MIN? (A two's complement architecture in which
> the representation where the sign bit is set and all other bits is unset
> is trap representation is legal according to the C standard. Does
> someone know whether it would also be legal according to POSIX?)
>

I think we already rely on that pretty heavily in our code.

>From 91ee53a5a46dc15cbbc4d463d6057ffa483b0625 Mon Sep 17 00:00:00 2001
From: Lynne 
Date: Sun, 3 May 2020 21:17:33 +0100
Subject: [PATCH] lavc/bsf: add an Opus metadata bitstream filter

The only adjustable field is the gain. Some ripping/transcoding programs
have started to use it.
---
 libavcodec/Makefile|  1 +
 libavcodec/bitstream_filters.c |  1 +
 libavcodec/opus_metadata_bsf.c | 72 ++
 3 files changed, 74 insertions(+)
 create mode 100644 libavcodec/opus_metadata_bsf.c

diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 28076c2c83..cf72f55aff 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -1116,6 +1116,7 @@ OBJS-$(CONFIG_MP3_HEADER_DECOMPRESS_BSF)  += mp3_header_decompress_bsf.o \
 OBJS-$(CONFIG_MPEG2_METADATA_BSF) += mpeg2_metadata_bsf.o
 OBJS-$(CONFIG_NOISE_BSF)  += noise_bsf.o
 OBJS-$(CONFIG_NULL_BSF)   += null_bsf.o
+OBJS-$(CONFIG_OPUS_METADATA_BSF)  += opus_metadata_bsf.o
 OBJS-$(CONFIG_PRORES_METADATA_BSF)+= prores_metadata_bsf.o
 OBJS-$(CONFIG_REMOVE_EXTRADATA_BSF)   += remove_extradata_bsf.o
 OBJS-$(CONFIG_TEXT2MOVSUB_BSF)+= movsub_bsf.o
diff --git a/libavcodec/bitstream_filters.c b/libavcodec/bitstream_filters.c
index 6b5ffe4d70..f1b24baa53 100644
--- a/libavcodec/bitstream_filters.c
+++ b/libavcodec/bitstream_filters.c
@@ -49,6 +49,7 @@ extern const AVBitStreamFilter ff_mpeg4_unpack_bframes_bsf;
 extern const AVBitStreamFilter ff_mov2textsub_bsf;
 extern const AVBitStreamFilter ff_noise_bsf;
 extern const AVBitStreamFilter ff_null_bsf;
+extern const AVBitStreamFilter ff_opus_metadata_bsf;
 extern const AVBitStreamFilter ff_prores_metadata_bsf;
 extern const AVBitStreamFilter ff_remove_extradata_bsf;
 extern const AVBitStreamFilter ff_text2movsub_bsf;
diff --git a/libavcodec/opus_metadata_bsf.c b/libavcodec/opus_metadata_bsf.c
new file mode 100644
index 00..867ad830d3
--- /dev/null
+++ b/libavcodec/opus_metadata_bsf.c
@@ -0,0 +1,72 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "bsf.h"
+#include "libavutil/intreadwrite.h"
+#include "libavutil/opt.h"
+
+typedef struct OpusBSFContext {
+const AVClass *class;
+int gain;
+} OpusBSFContext;
+
+static int opus_metadata_filter(AVBSFContext *bsfc, AVPacket *pkt)
+{
+return ff_bsf_get_packet_ref(bsfc, pkt);
+}
+
+static int opus_metadata_init(AVBSFContext *bsfc)
+{
+OpusBSFContext *s = bsfc->priv_data;
+
+if (bsfc->par_out->extradata_size < 19)
+return AVERROR_INVALIDDATA;
+
+AV_WL16(bsfc->par_out->extradata + 16, s->gain);
+
+return 0;
+}
+
+#define OFFSET(x) offsetof(OpusBSFContext, x)
+#define FLAGS (AV_OPT_FLAG_AUDIO_PARAM|AV_OPT_FLAG_BSF_PARAM)
+static const AVOption opus_metadata_options[] = {
+{ "gain", "Gain, actual amplification is pow(10, gain/(20.0*256))", OFFSET(gain),
+  AV_OPT_TYPE_INT, { .i64 = 0 }, -(INT16_MAX + 1), INT16_MAX, .flags = FLAGS },
+
+{ NULL },
+};
+
+static const AVClass op

Re: [FFmpeg-devel] [PATCH] lavc/bsf: add an Opus metadata bitstream filter

2020-05-05 Thread Lynne 
May 3, 2020, 21:21 by d...@lynne.ee:

> The only adjustable field is the gain. Some ripping/transcoding programs 
> have started to use it for replaygain adjustments.
>
> Patch attached.
>

Planning to push this tonight if there are no objections.
___
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] lavc/bsf: add an Opus metadata bitstream filter

2020-05-05 Thread Nicolas George 
Andreas Rheinhardt (12020-05-05):
> > +static const AVOption opus_metadata_options[] = {
> > +{ "gain", "Gain, actual amplification is pow(10, gain/(20.0*256))", 
> > OFFSET(gain),
> > +  AV_OPT_TYPE_INT, { .i64 = 0 }, -(INT16_MAX + 1), INT16_MAX, .flags = 
> > FLAGS },
> > +
> > +{ NULL },
> > +};
> 
> You are using an AV_OPT_TYPE_INT parameter, yet the actual type of the
> destination is int64_t. This won't work on big endian systems. Make gain
> an int.

Or make it a float and multiply it by 256 or 5120 before giving it to
libopus, possibly.

Regards,

-- 
  Nicolas George


signature.asc
Description: PGP signature
___
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] lavc/bsf: add an Opus metadata bitstream filter

2020-05-05 Thread Andreas Rheinhardt 
Lynne:
> The only adjustable field is the gain. Some ripping/transcoding programs 
> have started to use it for replaygain adjustments.
> 
> Patch attached.
> 
> >
> +typedef struct OpusBSFContext {
> +const AVClass *class;
> +int64_t gain;
> +} OpusBSFContext;
[...]
> 
> +static const AVOption opus_metadata_options[] = {
> +{ "gain", "Gain, actual amplification is pow(10, gain/(20.0*256))", 
> OFFSET(gain),
> +  AV_OPT_TYPE_INT, { .i64 = 0 }, -(INT16_MAX + 1), INT16_MAX, .flags = 
> FLAGS },
> +
> +{ NULL },
> +};

You are using an AV_OPT_TYPE_INT parameter, yet the actual type of the
destination is int64_t. This won't work on big endian systems. Make gain
an int.

- Andreas

PS: Do we actually support two's complement architectures were
-(INT16_MAX + 1) != INT16_MIN? (A two's complement architecture in which
the representation where the sign bit is set and all other bits is unset
is trap representation is legal according to the C standard. Does
someone know whether it would also be legal according to POSIX?)
___
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] lavc/bsf: add an Opus metadata bitstream filter

2020-05-03 Thread Lynne 
The only adjustable field is the gain. Some ripping/transcoding programs 
have started to use it for replaygain adjustments.

Patch attached.

>From 44f83786e057d468efee986ee865ba8c776ebe9b Mon Sep 17 00:00:00 2001
From: Lynne 
Date: Sun, 3 May 2020 21:17:33 +0100
Subject: [PATCH] lavc/bsf: add an Opus metadata bitstream filter

The only adjustable field is the gain. Some ripping/transcoding programs
have started to use it.
---
 libavcodec/Makefile|  1 +
 libavcodec/bitstream_filters.c |  1 +
 libavcodec/opus_metadata_bsf.c | 72 ++
 3 files changed, 74 insertions(+)
 create mode 100644 libavcodec/opus_metadata_bsf.c

diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 28076c2c83..cf72f55aff 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -1116,6 +1116,7 @@ OBJS-$(CONFIG_MP3_HEADER_DECOMPRESS_BSF)  += mp3_header_decompress_bsf.o \
 OBJS-$(CONFIG_MPEG2_METADATA_BSF) += mpeg2_metadata_bsf.o
 OBJS-$(CONFIG_NOISE_BSF)  += noise_bsf.o
 OBJS-$(CONFIG_NULL_BSF)   += null_bsf.o
+OBJS-$(CONFIG_OPUS_METADATA_BSF)  += opus_metadata_bsf.o
 OBJS-$(CONFIG_PRORES_METADATA_BSF)+= prores_metadata_bsf.o
 OBJS-$(CONFIG_REMOVE_EXTRADATA_BSF)   += remove_extradata_bsf.o
 OBJS-$(CONFIG_TEXT2MOVSUB_BSF)+= movsub_bsf.o
diff --git a/libavcodec/bitstream_filters.c b/libavcodec/bitstream_filters.c
index 6b5ffe4d70..f1b24baa53 100644
--- a/libavcodec/bitstream_filters.c
+++ b/libavcodec/bitstream_filters.c
@@ -49,6 +49,7 @@ extern const AVBitStreamFilter ff_mpeg4_unpack_bframes_bsf;
 extern const AVBitStreamFilter ff_mov2textsub_bsf;
 extern const AVBitStreamFilter ff_noise_bsf;
 extern const AVBitStreamFilter ff_null_bsf;
+extern const AVBitStreamFilter ff_opus_metadata_bsf;
 extern const AVBitStreamFilter ff_prores_metadata_bsf;
 extern const AVBitStreamFilter ff_remove_extradata_bsf;
 extern const AVBitStreamFilter ff_text2movsub_bsf;
diff --git a/libavcodec/opus_metadata_bsf.c b/libavcodec/opus_metadata_bsf.c
new file mode 100644
index 00..f0d794c1b3
--- /dev/null
+++ b/libavcodec/opus_metadata_bsf.c
@@ -0,0 +1,72 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "bsf.h"
+#include "libavutil/intreadwrite.h"
+#include "libavutil/opt.h"
+
+typedef struct OpusBSFContext {
+const AVClass *class;
+int64_t gain;
+} OpusBSFContext;
+
+static int opus_metadata_filter(AVBSFContext *bsfc, AVPacket *pkt)
+{
+return ff_bsf_get_packet_ref(bsfc, pkt);
+}
+
+static int opus_metadata_init(AVBSFContext *bsfc)
+{
+OpusBSFContext *s = bsfc->priv_data;
+
+if (bsfc->par_out->extradata_size < 19)
+return AVERROR_INVALIDDATA;
+
+AV_WL16(bsfc->par_out->extradata + 16, s->gain);
+
+return 0;
+}
+
+#define OFFSET(x) offsetof(OpusBSFContext, x)
+#define FLAGS (AV_OPT_FLAG_AUDIO_PARAM|AV_OPT_FLAG_BSF_PARAM)
+static const AVOption opus_metadata_options[] = {
+{ "gain", "Gain, actual amplification is pow(10, gain/(20.0*256))", OFFSET(gain),
+  AV_OPT_TYPE_INT, { .i64 = 0 }, -(INT16_MAX + 1), INT16_MAX, .flags = FLAGS },
+
+{ NULL },
+};
+
+static const AVClass opus_metadata_class = {
+.class_name = "opus_metadata_bsf",
+.item_name  = av_default_item_name,
+.option = opus_metadata_options,
+.version= LIBAVUTIL_VERSION_INT,
+};
+
+static const enum AVCodecID codec_ids[] = {
+AV_CODEC_ID_OPUS, AV_CODEC_ID_NONE,
+};
+
+const AVBitStreamFilter ff_opus_metadata_bsf = {
+.name   = "opus_metadata",
+.priv_data_size = sizeof(OpusBSFContext),
+.priv_class = &opus_metadata_class,
+.init   = &opus_metadata_init,
+.filter = &opus_metadata_filter,
+.codec_ids  = codec_ids,
+};
-- 
2.26.2

___
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".