Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH] libavformat/hlsenc.c: Populate OTI using AAC profile in write_codec_attr.
@ 2023-12-22 15:04 Romain Beauxis
  2023-12-28 15:05 ` Romain Beauxis
  2023-12-28 20:53 ` David Johansen
  0 siblings, 2 replies; 7+ messages in thread
From: Romain Beauxis @ 2023-12-22 15:04 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Romain Beauxis

This patch populates the third entry for HLS codec attribute using the
AAC profile.

The HLS specifications[1] require this digit to be the Object Type ID as
referred to in table 1.3 of ISO/IEC 14496-3:2009[2].

The numerical constants in the code refer to these OTIs minus one, as
documeted in commit 372597e[3], confirmed by comparing the values in the
code with the values in the table mentioned above.

Links:
1: https://datatracker.ietf.org/doc/html/rfc6381#section-3.3
2: https://csclub.uwaterloo.ca/~ehashman/ISO14496-3-2009.pdf
3: https://github.com/FFmpeg/FFmpeg/commit/372597e5381c097455a7b73849254d56083eb056

---
 libavformat/hlsenc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index 7049956dd7..2551bac6ae 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -418,8 +418,10 @@ static void write_codec_attr(AVStream *st, VariantStream *vs)
     } else if (st->codecpar->codec_id == AV_CODEC_ID_MP3) {
         snprintf(attr, sizeof(attr), "mp4a.40.34");
     } else if (st->codecpar->codec_id == AV_CODEC_ID_AAC) {
-        /* TODO : For HE-AAC, HE-AACv2, the last digit needs to be set to 5 and 29 respectively */
-        snprintf(attr, sizeof(attr), "mp4a.40.2");
+        if (st->codecpar->profile != AV_PROFILE_UNKNOWN)
+            snprintf(attr, sizeof(attr), "mp4a.40.%d", st->codecpar->profile+1);
+        else
+            goto fail;
     } else if (st->codecpar->codec_id == AV_CODEC_ID_AC3) {
         snprintf(attr, sizeof(attr), "ac-3");
     } else if (st->codecpar->codec_id == AV_CODEC_ID_EAC3) {
-- 
2.39.3 (Apple Git-145)

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [FFmpeg-devel] [PATCH] libavformat/hlsenc.c: Populate OTI using AAC profile in write_codec_attr.
  2023-12-22 15:04 [FFmpeg-devel] [PATCH] libavformat/hlsenc.c: Populate OTI using AAC profile in write_codec_attr Romain Beauxis
@ 2023-12-28 15:05 ` Romain Beauxis
  2023-12-28 20:53 ` David Johansen
  1 sibling, 0 replies; 7+ messages in thread
From: Romain Beauxis @ 2023-12-28 15:05 UTC (permalink / raw)
  To: ffmpeg-devel

Hey there!

Le ven. 22 déc. 2023 à 09:09, Romain Beauxis <toots@rastageeks.org> a écrit :
>
> This patch populates the third entry for HLS codec attribute using the
> AAC profile.
>
> The HLS specifications[1] require this digit to be the Object Type ID as
> referred to in table 1.3 of ISO/IEC 14496-3:2009[2].
>
> The numerical constants in the code refer to these OTIs minus one, as
> documented in commit 372597e[3], confirmed by comparing the values in the
> code with the values in the table mentioned above.
>
> Links:
> 1: https://datatracker.ietf.org/doc/html/rfc6381#section-3.3
> 2: https://csclub.uwaterloo.ca/~ehashman/ISO14496-3-2009.pdf
> 3: https://github.com/FFmpeg/FFmpeg/commit/372597e5381c097455a7b73849254d56083eb056

Anyone interested? I think that this is a pretty straight-forward
change that could potentially qualify as a bugfix for 6.1.1, after
all, this generates incorrect HLS playlist descriptions..

-- Romain

> ---
>  libavformat/hlsenc.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index 7049956dd7..2551bac6ae 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -418,8 +418,10 @@ static void write_codec_attr(AVStream *st, VariantStream *vs)
>      } else if (st->codecpar->codec_id == AV_CODEC_ID_MP3) {
>          snprintf(attr, sizeof(attr), "mp4a.40.34");
>      } else if (st->codecpar->codec_id == AV_CODEC_ID_AAC) {
> -        /* TODO : For HE-AAC, HE-AACv2, the last digit needs to be set to 5 and 29 respectively */
> -        snprintf(attr, sizeof(attr), "mp4a.40.2");
> +        if (st->codecpar->profile != AV_PROFILE_UNKNOWN)
> +            snprintf(attr, sizeof(attr), "mp4a.40.%d", st->codecpar->profile+1);
> +        else
> +            goto fail;
>      } else if (st->codecpar->codec_id == AV_CODEC_ID_AC3) {
>          snprintf(attr, sizeof(attr), "ac-3");
>      } else if (st->codecpar->codec_id == AV_CODEC_ID_EAC3) {
> --
> 2.39.3 (Apple Git-145)
>
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [FFmpeg-devel] [PATCH] libavformat/hlsenc.c: Populate OTI using AAC profile in write_codec_attr.
  2023-12-22 15:04 [FFmpeg-devel] [PATCH] libavformat/hlsenc.c: Populate OTI using AAC profile in write_codec_attr Romain Beauxis
  2023-12-28 15:05 ` Romain Beauxis
@ 2023-12-28 20:53 ` David Johansen
  2023-12-28 23:26   ` David Johansen
  1 sibling, 1 reply; 7+ messages in thread
From: David Johansen @ 2023-12-28 20:53 UTC (permalink / raw)
  To: FFmpeg development discussions and patches; +Cc: Romain Beauxis

On Fri, Dec 22, 2023 at 8:10 AM Romain Beauxis <toots@rastageeks.org> wrote:

> This patch populates the third entry for HLS codec attribute using the
> AAC profile.
>
> The HLS specifications[1] require this digit to be the Object Type ID as
> referred to in table 1.3 of ISO/IEC 14496-3:2009[2].
>
> The numerical constants in the code refer to these OTIs minus one, as
> documeted in commit 372597e[3], confirmed by comparing the values in the
> code with the values in the table mentioned above.
>
> Links:
> 1: https://datatracker.ietf.org/doc/html/rfc6381#section-3.3
> 2: https://csclub.uwaterloo.ca/~ehashman/ISO14496-3-2009.pdf
> 3:
> https://github.com/FFmpeg/FFmpeg/commit/372597e5381c097455a7b73849254d56083eb056
>
> ---
>  libavformat/hlsenc.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index 7049956dd7..2551bac6ae 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -418,8 +418,10 @@ static void write_codec_attr(AVStream *st,
> VariantStream *vs)
>      } else if (st->codecpar->codec_id == AV_CODEC_ID_MP3) {
>          snprintf(attr, sizeof(attr), "mp4a.40.34");
>      } else if (st->codecpar->codec_id == AV_CODEC_ID_AAC) {
> -        /* TODO : For HE-AAC, HE-AACv2, the last digit needs to be set to
> 5 and 29 respectively */
> -        snprintf(attr, sizeof(attr), "mp4a.40.2");
> +        if (st->codecpar->profile != AV_PROFILE_UNKNOWN)
> +            snprintf(attr, sizeof(attr), "mp4a.40.%d",
> st->codecpar->profile+1);
> +        else
> +            goto fail;
>      } else if (st->codecpar->codec_id == AV_CODEC_ID_AC3) {
>          snprintf(attr, sizeof(attr), "ac-3");
>      } else if (st->codecpar->codec_id == AV_CODEC_ID_EAC3) {
> --
> 2.39.3 (Apple Git-145)
>

I love this change, but it appears that st->codecpar->profile is always
AV_PROFILE_UNKNOWN when using libfdk_aac as the encoder. Any indications
where I should look for fix that so this can be used with that encoder?

Thanks,
Dave
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [FFmpeg-devel] [PATCH] libavformat/hlsenc.c: Populate OTI using AAC profile in write_codec_attr.
  2023-12-28 20:53 ` David Johansen
@ 2023-12-28 23:26   ` David Johansen
  2023-12-30 15:22     ` Romain Beauxis
  0 siblings, 1 reply; 7+ messages in thread
From: David Johansen @ 2023-12-28 23:26 UTC (permalink / raw)
  To: FFmpeg development discussions and patches; +Cc: Romain Beauxis

>
> I love this change, but it appears that st->codecpar->profile is always
> AV_PROFILE_UNKNOWN when using libfdk_aac as the encoder. Any indications
> where I should look for fix that so this can be used with that encoder?
>

It appears that the issue is that profile doesn't default to what's being
used so `--profile:a` has to be set explicitly with libfdk_aac and then it
works. Not sure if that's an issue worth fixing, but if someone points me
to where it needs to be done, then I'd be glad to take a look at fixing it
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [FFmpeg-devel] [PATCH] libavformat/hlsenc.c: Populate OTI using AAC profile in write_codec_attr.
  2023-12-28 23:26   ` David Johansen
@ 2023-12-30 15:22     ` Romain Beauxis
  2023-12-30 22:25       ` David Johansen
  0 siblings, 1 reply; 7+ messages in thread
From: Romain Beauxis @ 2023-12-30 15:22 UTC (permalink / raw)
  To: David Johansen; +Cc: FFmpeg development discussions and patches

Le jeu. 28 déc. 2023 à 17:26, David Johansen <davejohansen@gmail.com> a écrit :
>>
>> I love this change, but it appears that st->codecpar->profile is always AV_PROFILE_UNKNOWN when using libfdk_aac as the encoder. Any indications where I should look for fix that so this can be used with that encoder?
>
>
> It appears that the issue is that profile doesn't default to what's being used so `--profile:a` has to be set explicitly with libfdk_aac and then it works. Not sure if that's an issue worth fixing, but if someone points me to where it needs to be done, then I'd be glad to take a look at fixing it

This feels like a second, separate issue to me?

Maybe we could get these changes in first and then tackle it?

-- Romain
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [FFmpeg-devel] [PATCH] libavformat/hlsenc.c: Populate OTI using AAC profile in write_codec_attr.
  2023-12-30 15:22     ` Romain Beauxis
@ 2023-12-30 22:25       ` David Johansen
  2024-01-01 15:54         ` Romain Beauxis
  0 siblings, 1 reply; 7+ messages in thread
From: David Johansen @ 2023-12-30 22:25 UTC (permalink / raw)
  To: Romain Beauxis; +Cc: FFmpeg development discussions and patches

On Sat, Dec 30, 2023 at 8:23 AM Romain Beauxis <toots@rastageeks.org> wrote:

> Le jeu. 28 déc. 2023 à 17:26, David Johansen <davejohansen@gmail.com> a
> écrit :
> >>
> >> I love this change, but it appears that st->codecpar->profile is always
> AV_PROFILE_UNKNOWN when using libfdk_aac as the encoder. Any indications
> where I should look for fix that so this can be used with that encoder?
> >
> >
> > It appears that the issue is that profile doesn't default to what's
> being used so `--profile:a` has to be set explicitly with libfdk_aac and
> then it works. Not sure if that's an issue worth fixing, but if someone
> points me to where it needs to be done, then I'd be glad to take a look at
> fixing it
>
> This feels like a second, separate issue to me?
>
> Maybe we could get these changes in first and then tackle it?
>

But this is technically a breaking change, because it takes commands/uses
that currently work and changes them to no longer include CODECS since the
profile value is unknown by default
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [FFmpeg-devel] [PATCH] libavformat/hlsenc.c: Populate OTI using AAC profile in write_codec_attr.
  2023-12-30 22:25       ` David Johansen
@ 2024-01-01 15:54         ` Romain Beauxis
  0 siblings, 0 replies; 7+ messages in thread
From: Romain Beauxis @ 2024-01-01 15:54 UTC (permalink / raw)
  To: David Johansen; +Cc: FFmpeg development discussions and patches

Le sam. 30 déc. 2023 à 16:25, David Johansen <davejohansen@gmail.com> a écrit :
>
> On Sat, Dec 30, 2023 at 8:23 AM Romain Beauxis <toots@rastageeks.org> wrote:
>>
>> Le jeu. 28 déc. 2023 à 17:26, David Johansen <davejohansen@gmail.com> a écrit :
>> >>
>> >> I love this change, but it appears that st->codecpar->profile is always AV_PROFILE_UNKNOWN when using libfdk_aac as the encoder. Any indications where I should look for fix that so this can be used with that encoder?
>> >
>> >
>> > It appears that the issue is that profile doesn't default to what's being used so `--profile:a` has to be set explicitly with libfdk_aac and then it works. Not sure if that's an issue worth fixing, but if someone points me to where it needs to be done, then I'd be glad to take a look at fixing it
>>
>> This feels like a second, separate issue to me?
>>
>> Maybe we could get these changes in first and then tackle it?
>
>
> But this is technically a breaking change, because it takes commands/uses that currently work and changes them to no longer include CODECS since the profile value is unknown by default

Ha gotcha.

Yes, in this case we do need to send the previous value as fallback
when the profile is unknown.

Just sent an updated patch!

-- Romain
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-01-01 15:54 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-22 15:04 [FFmpeg-devel] [PATCH] libavformat/hlsenc.c: Populate OTI using AAC profile in write_codec_attr Romain Beauxis
2023-12-28 15:05 ` Romain Beauxis
2023-12-28 20:53 ` David Johansen
2023-12-28 23:26   ` David Johansen
2023-12-30 15:22     ` Romain Beauxis
2023-12-30 22:25       ` David Johansen
2024-01-01 15:54         ` Romain Beauxis

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git