[Bug 791674] Continued improvements to AOM plugin

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=791674

Sebastian Dröge (slomo) <slomo at coaxion.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #367432|none                        |needs-work
             status|                            |

--- Comment #9 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
Review of attachment 367432:
 --> (https://bugzilla.gnome.org/review?bug=791674&attachment=367432)

::: ext/aom/gstav1dec.c
@@ +193,3 @@
 gst_av1_dec_set_format (GstVideoDecoder * dec, GstVideoCodecState * state)
 {
+  GstAV1Dec *av1dec = GST_AV1_DEC (dec);

These changes seem like cleanup that should probably happen in a separate
commit. Usually we define a GST_AV1_DEC_CAST() for the direct cast without
runtime type checks

@@ +383,3 @@
+      GST_WARNING_OBJECT (av1dec, "%s", warn);
+    }
+    aom_img_free (img);

draining/finishing should both forward all pending frames downstream. flushing
should drop all pending frames and reset the codec.

::: ext/aom/gstav1enc.c
@@ +149,3 @@
+
+  av1enc->encoder_inited = FALSE;
+  av1enc->deadline = AOM_DL_GOOD_QUALITY;

You just add this here to remove it again in the next commit :)

@@ +262,3 @@
+  av1enc->aom_cfg.g_h = GST_VIDEO_INFO_HEIGHT (&av1enc->input_state->info);
+  av1enc->aom_cfg.g_timebase.num = 1;
+  av1enc->aom_cfg.g_timebase.den = 90000;

Why? Also these changes seem all unrelated to what you wrote in the commit
message.

Doing it like in vpxenc would make sense here IMHO

@@ -349,3 @@
     flags |= AOM_EFLAG_FORCE_KF;
   }
-  av1enc->keyframe_dist++;

You're removing the regular keyframes here. Is there automatic
keyframe-creation in the codec now? Can a max keyframe distance be configured?

@@ +379,3 @@
+
+    if (duration > 0) {
+      av1enc->last_pts += frame->duration;

What do we need the last_pts for? By adding up frame durations you add up
rounding errors

@@ +434,3 @@
+  }
+
+  return gst_av1_enc_process (av1enc);

here's only ever a single frame left in the codec? Shouldn't this be a loop?

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.