diff mbox series

[FFmpeg-devel] Extract av_hls_codec_attr

Message ID 20231030010549.59349-1-toots@rastageeks.org
State New
Headers show
Series [FFmpeg-devel] Extract av_hls_codec_attr | expand

Checks

Context Check Description
andriy/commit_msg_x86 warning The first line of the commit message must start with a context terminated by a colon and a space, for example "lavu/opt: " or "doc: ".
yinshiyou/commit_msg_loongarch64 warning The first line of the commit message must start with a context terminated by a colon and a space, for example "lavu/opt: " or "doc: ".
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Romain Beauxis Oct. 30, 2023, 1:05 a.m. UTC
The logic for extracting HLS codec attribute strings is very useful and
can be re-used in many different situations when working with HLS
streams using libavcodec/libavformat.

This patch extracts the function's code and places it into a publicly
available function.

---
 libavcodec/Makefile  |   2 +
 libavcodec/hls.c     | 105 +++++++++++++++++++++++++++++++++++++++++++
 libavcodec/hls.h     |  42 +++++++++++++++++
 libavformat/hlsenc.c |  83 +++-------------------------------
 4 files changed, 155 insertions(+), 77 deletions(-)
 create mode 100644 libavcodec/hls.c
 create mode 100644 libavcodec/hls.h

Comments

Michael Niedermayer Oct. 31, 2023, 1:28 p.m. UTC | #1
On Sun, Oct 29, 2023 at 08:05:50PM -0500, Romain Beauxis wrote:
> The logic for extracting HLS codec attribute strings is very useful and
> can be re-used in many different situations when working with HLS
> streams using libavcodec/libavformat.
> 
> This patch extracts the function's code and places it into a publicly
> available function.
> 
> ---
>  libavcodec/Makefile  |   2 +
>  libavcodec/hls.c     | 105 +++++++++++++++++++++++++++++++++++++++++++
>  libavcodec/hls.h     |  42 +++++++++++++++++
>  libavformat/hlsenc.c |  83 +++-------------------------------

you cannot call ff_* functions across libs

they need to start with av* / avpriv*


[...]
> +              rbsp_buf = ff_nal_unit_extract_rbsp(data, remain_size, &rbsp_size, 0);

libavcodec/libavcodec.so: undefined reference to `ff_nal_unit_extract_rbsp'
clang: error: linker command failed with exit code 1 (use -v to see invocation)
Makefile:133: recipe for target 'ffmpeg_g' failed
make: *** [ffmpeg_g] Error 1
make: *** Waiting for unfinished jobs....
libavcodec/libavcodec.so: undefined reference to `ff_nal_unit_extract_rbsp'
clang: error: linker command failed with exit code 1 (use -v to see invocation)
Makefile:133: recipe for target 'ffplay_g' failed
make: *** [ffplay_g] Error 1

thx

[...]
Zhao Zhili Oct. 31, 2023, 4:47 p.m. UTC | #2
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Romain Beauxis
> Sent: 2023年10月30日 9:06
> To: ffmpeg-devel@ffmpeg.org
> Cc: Romain Beauxis <toots@rastageeks.org>
> Subject: [FFmpeg-devel] [PATCH] Extract av_hls_codec_attr
> 
> The logic for extracting HLS codec attribute strings is very useful and
> can be re-used in many different situations when working with HLS
> streams using libavcodec/libavformat.
> 
> This patch extracts the function's code and places it into a publicly
> available function.

I don't think the implementation is complete enough to be exported as
an API. And the ad-hoc API needs some design too.

> 
> ---
>  libavcodec/Makefile  |   2 +
>  libavcodec/hls.c     | 105 +++++++++++++++++++++++++++++++++++++++++++
>  libavcodec/hls.h     |  42 +++++++++++++++++
>  libavformat/hlsenc.c |  83 +++-------------------------------
>  4 files changed, 155 insertions(+), 77 deletions(-)
>  create mode 100644 libavcodec/hls.c
>  create mode 100644 libavcodec/hls.h
>
Romain Beauxis Nov. 3, 2023, 5:20 a.m. UTC | #3
Le mar. 31 oct. 2023 à 08:28, Michael Niedermayer
<michael@niedermayer.cc> a écrit :
>
> On Sun, Oct 29, 2023 at 08:05:50PM -0500, Romain Beauxis wrote:
> > The logic for extracting HLS codec attribute strings is very useful and
> > can be re-used in many different situations when working with HLS
> > streams using libavcodec/libavformat.
> >
> > This patch extracts the function's code and places it into a publicly
> > available function.
> >
> > ---
> >  libavcodec/Makefile  |   2 +
> >  libavcodec/hls.c     | 105 +++++++++++++++++++++++++++++++++++++++++++
> >  libavcodec/hls.h     |  42 +++++++++++++++++
> >  libavformat/hlsenc.c |  83 +++-------------------------------
>
> you cannot call ff_* functions across libs
>
> they need to start with av* / avpriv*

Thanks for the review! Updated patch submitted.

> [...]
> > +              rbsp_buf = ff_nal_unit_extract_rbsp(data, remain_size, &rbsp_size, 0);
>
> libavcodec/libavcodec.so: undefined reference to `ff_nal_unit_extract_rbsp'
> clang: error: linker command failed with exit code 1 (use -v to see invocation)
> Makefile:133: recipe for target 'ffmpeg_g' failed
> make: *** [ffmpeg_g] Error 1
> make: *** Waiting for unfinished jobs....
> libavcodec/libavcodec.so: undefined reference to `ff_nal_unit_extract_rbsp'
> clang: error: linker command failed with exit code 1 (use -v to see invocation)
> Makefile:133: recipe for target 'ffplay_g' failed
> make: *** [ffplay_g] Error 1
>
> thx
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> If a bugfix only changes things apparently unrelated to the bug with no
> further explanation, that is a good sign that the bugfix is wrong.
> _______________________________________________
> 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".
Romain Beauxis Nov. 3, 2023, 5:21 a.m. UTC | #4
Le mar. 31 oct. 2023 à 11:47, Zhao Zhili <quinkblack@foxmail.com> a écrit :
>
>
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Romain Beauxis
> > Sent: 2023年10月30日 9:06
> > To: ffmpeg-devel@ffmpeg.org
> > Cc: Romain Beauxis <toots@rastageeks.org>
> > Subject: [FFmpeg-devel] [PATCH] Extract av_hls_codec_attr
> >
> > The logic for extracting HLS codec attribute strings is very useful and
> > can be re-used in many different situations when working with HLS
> > streams using libavcodec/libavformat.
> >
> > This patch extracts the function's code and places it into a publicly
> > available function.
>
> I don't think the implementation is complete enough to be exported as
> an API. And the ad-hoc API needs some design too.

I am not in a position to dispute this assessment but I would say it
could be a "build it and they will come" kind of situation (or chicken
and egg too).

> >
> > ---
> >  libavcodec/Makefile  |   2 +
> >  libavcodec/hls.c     | 105 +++++++++++++++++++++++++++++++++++++++++++
> >  libavcodec/hls.h     |  42 +++++++++++++++++
> >  libavformat/hlsenc.c |  83 +++-------------------------------
> >  4 files changed, 155 insertions(+), 77 deletions(-)
> >  create mode 100644 libavcodec/hls.c
> >  create mode 100644 libavcodec/hls.h
> >
>
>
> _______________________________________________
> 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".
Zhao Zhili Nov. 3, 2023, 5:28 a.m. UTC | #5
> 在 2023年11月3日,下午1:21,Romain Beauxis <romain.beauxis@gmail.com> 写道:
> 
> Le mar. 31 oct. 2023 à 11:47, Zhao Zhili <quinkblack@foxmail.com> a écrit :
>> 
>> 
>>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Romain Beauxis
>>> Sent: 2023年10月30日 9:06
>>> To: ffmpeg-devel@ffmpeg.org
>>> Cc: Romain Beauxis <toots@rastageeks.org>
>>> Subject: [FFmpeg-devel] [PATCH] Extract av_hls_codec_attr
>>> 
>>> The logic for extracting HLS codec attribute strings is very useful and
>>> can be re-used in many different situations when working with HLS
>>> streams using libavcodec/libavformat.
>>> 
>>> This patch extracts the function's code and places it into a publicly
>>> available function.
>> 
>> I don't think the implementation is complete enough to be exported as
>> an API. And the ad-hoc API needs some design too.
> 
> I am not in a position to dispute this assessment but I would say it
> could be a "build it and they will come" kind of situation (or chicken
> and egg too).

Maybe it’s fine for internal implementation, but dangerous for API.

> 
>>> 
>>> ---
>>> libavcodec/Makefile  |   2 +
>>> libavcodec/hls.c     | 105 +++++++++++++++++++++++++++++++++++++++++++
>>> libavcodec/hls.h     |  42 +++++++++++++++++
>>> libavformat/hlsenc.c |  83 +++-------------------------------
>>> 4 files changed, 155 insertions(+), 77 deletions(-)
>>> create mode 100644 libavcodec/hls.c
>>> create mode 100644 libavcodec/hls.h
>>> 
>> 
>> 
>> _______________________________________________
>> 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".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-dev
diff mbox series

Patch

diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 580a8d6b54..b3b2b18980 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -16,6 +16,7 @@  HEADERS = ac3_parser.h                                                  \
           dirac.h                                                       \
           dv_profile.h                                                  \
           dxva2.h                                                       \
+          hls.h                                                         \
           jni.h                                                         \
           mediacodec.h                                                  \
           packet.h                                                      \
@@ -47,6 +48,7 @@  OBJS = ac3_parser.o                                                     \
        get_buffer.o                                                     \
        imgconvert.o                                                     \
        jni.o                                                            \
+       hls.o                                                            \
        mathtables.o                                                     \
        mediacodec.o                                                     \
        mpeg12framerate.o                                                \
diff --git a/libavcodec/hls.c b/libavcodec/hls.c
new file mode 100644
index 0000000000..59ab0c4819
--- /dev/null
+++ b/libavcodec/hls.c
@@ -0,0 +1,105 @@ 
+/*
+ * HLS public API
+ *
+ * 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 "hls.h"
+#include "libavutil/intreadwrite.h"
+#include "libavformat/avc.h"
+
+int av_hls_codec_attr(AVStream *st, char *attr, size_t attr_len) {
+  if (st->codecpar->codec_id == AV_CODEC_ID_H264) {
+      uint8_t *data = st->codecpar->extradata;
+      if (data) {
+          const uint8_t *p;
+
+          if (AV_RB32(data) == 0x01 && (data[4] & 0x1F) == 7)
+              p = &data[5];
+          else if (AV_RB24(data) == 0x01 && (data[3] & 0x1F) == 7)
+              p = &data[4];
+          else if (data[0] == 0x01)  /* avcC */
+              p = &data[1];
+          else
+              return AVERROR_INVALIDDATA;
+          snprintf(attr, attr_len,
+                   "avc1.%02x%02x%02x", p[0], p[1], p[2]);
+      } else {
+          return AVERROR_INVALIDDATA;
+      }
+  } else if (st->codecpar->codec_id == AV_CODEC_ID_HEVC) {
+      uint8_t *data = st->codecpar->extradata;
+      int profile = AV_PROFILE_UNKNOWN;
+      int level = AV_LEVEL_UNKNOWN;
+
+      if (st->codecpar->profile != AV_PROFILE_UNKNOWN)
+          profile = st->codecpar->profile;
+      if (st->codecpar->level != AV_LEVEL_UNKNOWN)
+          level = st->codecpar->level;
+
+      /* check the boundary of data which from current position is small than extradata_size */
+      while (data && (data - st->codecpar->extradata + 19) < st->codecpar->extradata_size) {
+          /* get HEVC SPS NAL and seek to profile_tier_level */
+          if (!(data[0] | data[1] | data[2]) && data[3] == 1 && ((data[4] & 0x7E) == 0x42)) {
+              uint8_t *rbsp_buf;
+              int remain_size = 0;
+              int rbsp_size = 0;
+              /* skip start code + nalu header */
+              data += 6;
+              /* process by reference General NAL unit syntax */
+              remain_size = st->codecpar->extradata_size - (data - st->codecpar->extradata);
+              rbsp_buf = ff_nal_unit_extract_rbsp(data, remain_size, &rbsp_size, 0);
+              if (!rbsp_buf)
+                  return AVERROR_INVALIDDATA;
+              if (rbsp_size < 13) {
+                  av_freep(&rbsp_buf);
+                  break;
+              }
+              /* skip sps_video_parameter_set_id   u(4),
+               *      sps_max_sub_layers_minus1    u(3),
+               *  and sps_temporal_id_nesting_flag u(1) */
+              profile = rbsp_buf[1] & 0x1f;
+              /* skip 8 + 8 + 32 + 4 + 43 + 1 bit */
+              level = rbsp_buf[12];
+              av_freep(&rbsp_buf);
+              break;
+          }
+          data++;
+      }
+      if (st->codecpar->codec_tag == MKTAG('h','v','c','1') &&
+          profile != AV_PROFILE_UNKNOWN &&
+          level != AV_LEVEL_UNKNOWN) {
+          snprintf(attr, attr_len, "%s.%d.4.L%d.B01", av_fourcc2str(st->codecpar->codec_tag), profile, level);
+      } else
+          return AVERROR_INVALIDDATA;
+  } else if (st->codecpar->codec_id == AV_CODEC_ID_MP2) {
+      snprintf(attr, attr_len, "mp4a.40.33");
+  } else if (st->codecpar->codec_id == AV_CODEC_ID_MP3) {
+      snprintf(attr, attr_len, "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, attr_len, "mp4a.40.2");
+  } else if (st->codecpar->codec_id == AV_CODEC_ID_AC3) {
+      snprintf(attr, attr_len, "ac-3");
+  } else if (st->codecpar->codec_id == AV_CODEC_ID_EAC3) {
+      snprintf(attr, attr_len, "ec-3");
+  } else {
+      return AVERROR_INVALIDDATA;
+  }
+
+  return 0;
+}
diff --git a/libavcodec/hls.h b/libavcodec/hls.h
new file mode 100644
index 0000000000..c9696386b7
--- /dev/null
+++ b/libavcodec/hls.h
@@ -0,0 +1,42 @@ 
+/*
+ * HLS public API
+ *
+ * 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
+ */
+
+/**
+ * @file
+ * A public API for HLS codec information
+ */
+
+#ifndef AVCODEC_HLS_H
+#define AVCODEC_HLS_H
+
+#include "libavformat/avformat.h"
+
+/**
+ * Write a NULL-terminated string representing the codec attributes
+ * associated with the given AVStream.
+ *
+ * @param st       stream to extract codec attributes from
+ * @param attr     buffer to write to wanted payload size
+ * @param attr_len size of the buffer
+ * @return 0 on success, a negative AVERROR code on failure
+ */
+int av_hls_codec_attr(AVStream *st, char *attr, size_t attr_len);
+
+#endif
diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index 4ef84c05c1..1c0a35c36d 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -40,6 +40,7 @@ 
 #include "libavutil/time_internal.h"
 
 #include "libavcodec/defs.h"
+#include "libavcodec/hls.h"
 
 #include "avformat.h"
 #include "avio_internal.h"
@@ -344,89 +345,17 @@  static void write_codec_attr(AVStream *st, VariantStream *vs)
 {
     int codec_strlen = strlen(vs->codec_attr);
     char attr[32];
+    int ret;
 
     if (st->codecpar->codec_type == AVMEDIA_TYPE_SUBTITLE)
         return;
     if (vs->attr_status == CODEC_ATTRIBUTE_WILL_NOT_BE_WRITTEN)
         return;
 
-    if (st->codecpar->codec_id == AV_CODEC_ID_H264) {
-        uint8_t *data = st->codecpar->extradata;
-        if (data) {
-            const uint8_t *p;
-
-            if (AV_RB32(data) == 0x01 && (data[4] & 0x1F) == 7)
-                p = &data[5];
-            else if (AV_RB24(data) == 0x01 && (data[3] & 0x1F) == 7)
-                p = &data[4];
-            else if (data[0] == 0x01)  /* avcC */
-                p = &data[1];
-            else
-                goto fail;
-            snprintf(attr, sizeof(attr),
-                     "avc1.%02x%02x%02x", p[0], p[1], p[2]);
-        } else {
-            goto fail;
-        }
-    } else if (st->codecpar->codec_id == AV_CODEC_ID_HEVC) {
-        uint8_t *data = st->codecpar->extradata;
-        int profile = AV_PROFILE_UNKNOWN;
-        int level = AV_LEVEL_UNKNOWN;
-
-        if (st->codecpar->profile != AV_PROFILE_UNKNOWN)
-            profile = st->codecpar->profile;
-        if (st->codecpar->level != AV_LEVEL_UNKNOWN)
-            level = st->codecpar->level;
-
-        /* check the boundary of data which from current position is small than extradata_size */
-        while (data && (data - st->codecpar->extradata + 19) < st->codecpar->extradata_size) {
-            /* get HEVC SPS NAL and seek to profile_tier_level */
-            if (!(data[0] | data[1] | data[2]) && data[3] == 1 && ((data[4] & 0x7E) == 0x42)) {
-                uint8_t *rbsp_buf;
-                int remain_size = 0;
-                int rbsp_size = 0;
-                /* skip start code + nalu header */
-                data += 6;
-                /* process by reference General NAL unit syntax */
-                remain_size = st->codecpar->extradata_size - (data - st->codecpar->extradata);
-                rbsp_buf = ff_nal_unit_extract_rbsp(data, remain_size, &rbsp_size, 0);
-                if (!rbsp_buf)
-                    return;
-                if (rbsp_size < 13) {
-                    av_freep(&rbsp_buf);
-                    break;
-                }
-                /* skip sps_video_parameter_set_id   u(4),
-                 *      sps_max_sub_layers_minus1    u(3),
-                 *  and sps_temporal_id_nesting_flag u(1) */
-                profile = rbsp_buf[1] & 0x1f;
-                /* skip 8 + 8 + 32 + 4 + 43 + 1 bit */
-                level = rbsp_buf[12];
-                av_freep(&rbsp_buf);
-                break;
-            }
-            data++;
-        }
-        if (st->codecpar->codec_tag == MKTAG('h','v','c','1') &&
-            profile != AV_PROFILE_UNKNOWN &&
-            level != AV_LEVEL_UNKNOWN) {
-            snprintf(attr, sizeof(attr), "%s.%d.4.L%d.B01", av_fourcc2str(st->codecpar->codec_tag), profile, level);
-        } else
-            goto fail;
-    } else if (st->codecpar->codec_id == AV_CODEC_ID_MP2) {
-        snprintf(attr, sizeof(attr), "mp4a.40.33");
-    } 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");
-    } 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) {
-        snprintf(attr, sizeof(attr), "ec-3");
-    } else {
-        goto fail;
-    }
+    ret = av_hls_codec_attr(st, attr, sizeof(attr));
+    if (ret < 0)
+      goto fail;
+
     // Don't write the same attribute multiple times
     if (!av_stristr(vs->codec_attr, attr)) {
         snprintf(vs->codec_attr + codec_strlen,