From ce9b412d0ed2cbfd0247430e6e806229c27fde46 Mon Sep 17 00:00:00 2001
From: Yohan Totting <tyohan@inlive.app>
Date: Fri, 14 Feb 2025 12:37:53 +0700
Subject: [PATCH] Fix the VPX encoder is not properly handling the frame
 timestamp (#606)

* Fix the VPX encoder is not properly handling the frame timestamp

* Apply suggestions from code review

Co-authored-by: Atsushi Watanabe <atsushi.w@ieee.org>

---------

Co-authored-by: Atsushi Watanabe <atsushi.w@ieee.org>
---
 pkg/codec/vpx/vpx.go      | 12 +++++-----
 pkg/codec/vpx/vpx_test.go | 48 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/pkg/codec/vpx/vpx.go b/pkg/codec/vpx/vpx.go
index a2fb67d..906c6e4 100644
--- a/pkg/codec/vpx/vpx.go
+++ b/pkg/codec/vpx/vpx.go
@@ -69,8 +69,8 @@ type encoder struct {
 	cfg             *C.vpx_codec_enc_cfg_t
 	r               video.Reader
 	frameIndex      int
-	tStart          int
-	tLastFrame      int
+	tStart          time.Time
+	tLastFrame      time.Time
 	frame           []byte
 	deadline        int
 	requireKeyFrame bool
@@ -198,7 +198,7 @@ func newEncoder(r video.Reader, p prop.Media, params Params, codecIface *C.vpx_c
 	); ec != 0 {
 		return nil, fmt.Errorf("vpx_codec_enc_init failed (%d)", ec)
 	}
-	t0 := time.Now().Nanosecond() / 1000000
+	t0 := time.Now()
 	return &encoder{
 		r:          video.ToI420(r),
 		codec:      codec,
@@ -233,7 +233,7 @@ func (e *encoder) Read() ([]byte, func(), error) {
 	e.raw.stride[1] = C.int(yuvImg.CStride)
 	e.raw.stride[2] = C.int(yuvImg.CStride)
 
-	t := time.Now().Nanosecond() / 1000000
+	t := time.Now()
 
 	if e.cfg.g_w != C.uint(width) || e.cfg.g_h != C.uint(height) {
 		e.cfg.g_w, e.cfg.g_h = C.uint(width), C.uint(height)
@@ -252,7 +252,7 @@ func (e *encoder) Read() ([]byte, func(), error) {
 		e.raw.d_w, e.raw.d_h = C.uint(width), C.uint(height)
 	}
 
-	duration := t - e.tLastFrame
+	duration := t.Sub(e.tLastFrame).Microseconds()
 	// VPX doesn't allow 0 duration. If 0 is given, vpx_codec_encode will fail with VPX_CODEC_INVALID_PARAM.
 	// 0 duration is possible because mediadevices first gets the frame meta data by reading from the source,
 	// and consequently the codec will read the first frame from the buffer. This means the first frame won't
@@ -267,7 +267,7 @@ func (e *encoder) Read() ([]byte, func(), error) {
 	}
 	if ec := C.encode_wrapper(
 		e.codec, e.raw,
-		C.long(t-e.tStart), C.ulong(duration), C.long(flags), C.ulong(e.deadline),
+		C.long(t.Sub(e.tStart).Microseconds()), C.ulong(duration), C.long(flags), C.ulong(e.deadline),
 		(*C.uchar)(&yuvImg.Y[0]), (*C.uchar)(&yuvImg.Cb[0]), (*C.uchar)(&yuvImg.Cr[0]),
 	); ec != C.VPX_CODEC_OK {
 		return nil, func() {}, fmt.Errorf("vpx_codec_encode failed (%d)", ec)
diff --git a/pkg/codec/vpx/vpx_test.go b/pkg/codec/vpx/vpx_test.go
index 53010fa..94b73a2 100644
--- a/pkg/codec/vpx/vpx_test.go
+++ b/pkg/codec/vpx/vpx_test.go
@@ -1,10 +1,12 @@
 package vpx
 
 import (
+	"context"
 	"image"
 	"io"
 	"sync/atomic"
 	"testing"
+	"time"
 
 	"github.com/pion/mediadevices/pkg/codec"
 	"github.com/pion/mediadevices/pkg/codec/internal/codectest"
@@ -245,3 +247,49 @@ func TestShouldImplementKeyFrameControl(t *testing.T) {
 		t.Error()
 	}
 }
+
+func TestEncoderFrameMonotonic(t *testing.T) {
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+
+	params, err := NewVP8Params()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	encoder, err := params.BuildVideoEncoder(
+		video.ReaderFunc(func() (image.Image, func(), error) {
+			return image.NewYCbCr(
+				image.Rect(0, 0, 320, 240),
+				image.YCbCrSubsampleRatio420,
+			), func() {}, nil
+		},
+		), prop.Media{
+			Video: prop.Video{
+				Width:       320,
+				Height:      240,
+				FrameRate:   30,
+				FrameFormat: frame.FormatI420,
+			},
+		})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	ticker := time.NewTicker(33 * time.Millisecond)
+	defer ticker.Stop()
+	ctxx, cancell := context.WithCancel(ctx)
+	defer cancell()
+	for {
+		select {
+		case <-ctxx.Done():
+			return
+		case <-ticker.C:
+			_, rel, err := encoder.Read()
+			if err != nil {
+				t.Fatal(err)
+			}
+			rel()
+		}
+	}
+}