diff mbox

[FFmpeg-devel,v2,6/9] cbs_av1: Implement parser entrypoint

Message ID 20190401233940.5941-6-sw@jkqxz.net
State New
Headers show

Commit Message

Mark Thompson April 1, 2019, 11:39 p.m. UTC 
---
Unsure about this one - while the patch is short, it's rather invasive in a pretty ugly way with how it abuses the read call.  It will still do allocations for the decomposition because of that, even though we don't really want them.

Any ideas welcome.


 libavcodec/cbs_av1.c | 89 +++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 79 insertions(+), 10 deletions(-)

Comments

James Almer April 2, 2019, 6:36 p.m. UTC | #1 
On 4/1/2019 8:39 PM, Mark Thompson wrote:
> ---
> Unsure about this one - while the patch is short, it's rather invasive in a pretty ugly way with how it abuses the read call.  It will still do allocations for the decomposition because of that, even though we don't really want them.
> 
> Any ideas welcome.

How about copying most of cbs_av1_read_unit() into cbs_av1_parse_obu(),
factorizing the parts that can be shared (like pretty much everything
before the switch), without the call to ff_cbs_alloc_unit_content() and
only bothering to parse Sequence Header, Frame Header, Frame (Only the
header, and not the Tile Group part), using the needed structs on stack?
You wouldn't have to worry about cbs_av1_ref_tile_data() being called
with unit->data_ref as NULL at all.
James Almer April 2, 2019, 6:45 p.m. UTC | #2 
On 4/2/2019 3:36 PM, James Almer wrote:
> On 4/1/2019 8:39 PM, Mark Thompson wrote:
>> ---
>> Unsure about this one - while the patch is short, it's rather invasive in a pretty ugly way with how it abuses the read call.  It will still do allocations for the decomposition because of that, even though we don't really want them.
>>
>> Any ideas welcome.
> 
> How about copying most of cbs_av1_read_unit() into cbs_av1_parse_obu(),
> factorizing the parts that can be shared (like pretty much everything
> before the switch), without the call to ff_cbs_alloc_unit_content() and
> only bothering to parse Sequence Header, Frame Header, Frame (Only the
> header, and not the Tile Group part), using the needed structs on stack?
> You wouldn't have to worry about cbs_av1_ref_tile_data() being called
> with unit->data_ref as NULL at all.

So Tile Groups probably need to be parsed to set seen_frame_header back
to 0 before the next frame, but you can still skip the
cbs_av1_ref_tile_data() calls with the above suggestion.
James Almer April 2, 2019, 6:54 p.m. UTC | #3 
On 4/1/2019 8:39 PM, Mark Thompson wrote:
> +static int cbs_av1_parse_obu(CodedBitstreamContext *ctx,
> +                             void *priv, int obu_type,
> +                             const uint8_t *data, size_t data_size)
> +{
> +    CodedBitstreamUnit unit;
> +    int err;
> +
> +    // These OBU types will not affect parsing.
> +    if (obu_type == AV1_OBU_METADATA  ||
> +        obu_type == AV1_OBU_TILE_LIST ||
> +        obu_type == AV1_OBU_PADDING)
> +        return 0;
> +
> +    unit = (CodedBitstreamUnit) {
> +        .type      = obu_type,
> +        .data      = (uint8_t*)data,
> +        .data_size = data_size,
> +    };
> +
> +    err = cbs_av1_read_unit(ctx, &unit);
> +    if (err >= 0 && priv) {
> +        AV1RawOBU *obu = unit.content;
> +        switch (obu->header.obu_type) {
> +        case AV1_OBU_FRAME_HEADER:
> +        case AV1_OBU_REDUNDANT_FRAME_HEADER:
> +            memcpy(priv, &obu->obu.frame_header,
> +                   sizeof(obu->obu.frame_header));
> +            break;
> +        case AV1_OBU_FRAME:
> +            memcpy(priv, &obu->obu.frame.header,
> +                   sizeof(obu->obu.frame.header));
> +            break;
> +        }

You should check that it's the frame with show_frame == 1 or
show_existing_frame == 1 before copying, otherwise this depends on
Temporal Units with several frames having the visible one at the end.
I don't think the spec enforces ordering in that regard.
diff mbox

Patch

diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c
index 02f168b58d..0b54b5820c 100644
--- a/libavcodec/cbs_av1.c
+++ b/libavcodec/cbs_av1.c
@@ -776,13 +776,15 @@  static int cbs_av1_get_relative_dist(const AV1RawSequenceHeader *seq,
 #undef byte_alignment
 
 
-static int cbs_av1_split_fragment(CodedBitstreamContext *ctx,
-                                  CodedBitstreamFragment *frag,
-                                  int header)
+typedef int (*cbs_av1_split_obu_callback)(CodedBitstreamContext *ctx,
+                                          void *priv, int obu_type,
+                                          const uint8_t *data, size_t size);
+
+static int cbs_av1_split_obus(CodedBitstreamContext *ctx,
+                              void *priv, cbs_av1_split_obu_callback cb,
+                              const uint8_t *data, size_t size)
 {
     GetBitContext gbc;
-    uint8_t *data;
-    size_t size;
     uint64_t obu_length;
     int pos, err, trace;
 
@@ -790,9 +792,6 @@  static int cbs_av1_split_fragment(CodedBitstreamContext *ctx,
     trace = ctx->trace_enable;
     ctx->trace_enable = 0;
 
-    data = frag->data;
-    size = frag->data_size;
-
     if (INT_MAX / 8 < size) {
         av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid fragment: "
                "too large (%"SIZE_SPECIFIER" bytes).\n", size);
@@ -837,8 +836,7 @@  static int cbs_av1_split_fragment(CodedBitstreamContext *ctx,
             goto fail;
         }
 
-        err = ff_cbs_insert_unit_data(ctx, frag, -1, header.obu_type,
-                                      data, obu_length, frag->data_ref);
+        err = cb(ctx, priv, header.obu_type, data, obu_length);
         if (err < 0)
             goto fail;
 
@@ -852,6 +850,23 @@  fail:
     return err;
 }
 
+static int cbs_av1_insert_obu(CodedBitstreamContext *ctx,
+                              void *priv, int obu_type,
+                              const uint8_t *data, size_t size)
+{
+    CodedBitstreamFragment *frag = priv;
+    return ff_cbs_insert_unit_data(ctx, frag, -1, obu_type,
+                                   (uint8_t*)data, size, frag->data_ref);
+}
+
+static int cbs_av1_split_fragment(CodedBitstreamContext *ctx,
+                                  CodedBitstreamFragment *frag,
+                                  int header)
+{
+    return cbs_av1_split_obus(ctx, frag, &cbs_av1_insert_obu,
+                              frag->data, frag->data_size);
+}
+
 static void cbs_av1_free_tile_data(AV1RawTileData *td)
 {
     av_buffer_unref(&td->data_ref);
@@ -895,6 +910,12 @@  static int cbs_av1_ref_tile_data(CodedBitstreamContext *ctx,
 {
     int pos;
 
+    if (!unit->data_ref) {
+        // Not refcounted - only parsing headers, so tile data will
+        // not be needed.
+        return 0;
+    }
+
     pos = get_bits_count(gbc);
     if (pos >= 8 * unit->data_size) {
         av_log(ctx->log_ctx, AV_LOG_ERROR, "Bitstream ended before "
@@ -1324,6 +1345,52 @@  static void cbs_av1_close(CodedBitstreamContext *ctx)
     av_freep(&priv->write_buffer);
 }
 
+static int cbs_av1_parse_obu(CodedBitstreamContext *ctx,
+                             void *priv, int obu_type,
+                             const uint8_t *data, size_t data_size)
+{
+    CodedBitstreamUnit unit;
+    int err;
+
+    // These OBU types will not affect parsing.
+    if (obu_type == AV1_OBU_METADATA  ||
+        obu_type == AV1_OBU_TILE_LIST ||
+        obu_type == AV1_OBU_PADDING)
+        return 0;
+
+    unit = (CodedBitstreamUnit) {
+        .type      = obu_type,
+        .data      = (uint8_t*)data,
+        .data_size = data_size,
+    };
+
+    err = cbs_av1_read_unit(ctx, &unit);
+    if (err >= 0 && priv) {
+        AV1RawOBU *obu = unit.content;
+        switch (obu->header.obu_type) {
+        case AV1_OBU_FRAME_HEADER:
+        case AV1_OBU_REDUNDANT_FRAME_HEADER:
+            memcpy(priv, &obu->obu.frame_header,
+                   sizeof(obu->obu.frame_header));
+            break;
+        case AV1_OBU_FRAME:
+            memcpy(priv, &obu->obu.frame.header,
+                   sizeof(obu->obu.frame.header));
+            break;
+        }
+    }
+
+    av_buffer_unref(&unit.content_ref);
+
+    return err;
+}
+
+static int cbs_av1_parse_headers(CodedBitstreamContext *ctx, void *header,
+                                 const uint8_t *data, size_t size)
+{
+    return cbs_av1_split_obus(ctx, header, &cbs_av1_parse_obu, data, size);
+}
+
 const CodedBitstreamType ff_cbs_type_av1 = {
     .codec_id          = AV_CODEC_ID_AV1,
 
@@ -1335,4 +1402,6 @@  const CodedBitstreamType ff_cbs_type_av1 = {
     .assemble_fragment = &cbs_av1_assemble_fragment,
 
     .close             = &cbs_av1_close,
+
+    .parse_headers     = &cbs_av1_parse_headers,
 };