Repository: pion/mediadevices

Issue:
pkg/codec/vpx has codec plugin unrelated public type VpxImage
https://github.com/pion/mediadevices/pull/652 added `vpx.VpxImage` which looks independent of the `pkg/codec/vpx` package.

Packages under `pkg/codec` are plugins that implement the interfaces defined in `pkg/codec`; however, `VpxImage` is not.
Also, `unsafe.Pointer` is usually improper to be used in the public API in Go.

If it's used only by `bwe-test`, it should be defined in the `bwe-test` repository.


### ref
- https://github.com/pion/mediadevices/pull/652#discussion_r2449032222

Hints from issue comments:
@at-wat: @hanguyen-nuro (@yzhao-nuro @lkang-nuro) Could you clarify the reason that `VpxImage` should be defined in `pkg/codec/vpx`?

Also, I'd like you guys to assign at least one reviewer from outside your organization, especially when introducing a new API or changing an existing one, to ensure the API design is reviewed from a broad perspective.@hanguyen-nuro: Thank you for the suggestion! Makes sense to revert this PR as it is not needed anymore. I will keep that in mind going forward~ #662 @at-wat: Thank you for your cooperation!

Solution patch:
diff --git a/pkg/codec/vpx/bitrate_tracker.go b/pkg/codec/vpx/bitrate_tracker.go
deleted file mode 100644
index 8abe512..0000000
--- a/pkg/codec/vpx/bitrate_tracker.go
+++ /dev/null
@@ -1,48 +0,0 @@
-package vpx
-
-import (
-	"time"
-)
-
-type BitrateTracker struct {
-	windowSize time.Duration
-	buffer     []int
-	times      []time.Time
-}
-
-func NewBitrateTracker(windowSize time.Duration) *BitrateTracker {
-	return &BitrateTracker{
-		windowSize: windowSize,
-	}
-}
-
-func (bt *BitrateTracker) AddFrame(sizeBytes int, timestamp time.Time) {
-	bt.buffer = append(bt.buffer, sizeBytes)
-	bt.times = append(bt.times, timestamp)
-
-	// Remove old entries outside the window
-	cutoff := timestamp.Add(-bt.windowSize)
-	i := 0
-	for ; i < len(bt.times); i++ {
-		if bt.times[i].After(cutoff) {
-			break
-		}
-	}
-	bt.buffer = bt.buffer[i:]
-	bt.times = bt.times[i:]
-}
-
-func (bt *BitrateTracker) GetBitrate() float64 {
-	if len(bt.times) < 2 {
-		return 0
-	}
-	totalBytes := 0
-	for _, b := range bt.buffer {
-		totalBytes += b
-	}
-	duration := bt.times[len(bt.times)-1].Sub(bt.times[0]).Seconds()
-	if duration <= 0 {
-		return 0
-	}
-	return float64(totalBytes*8) / duration // bits per second
-}
diff --git a/pkg/codec/vpx/vpx_image.go b/pkg/codec/vpx/vpx_image.go
deleted file mode 100644
index 5ba4dae..0000000
--- a/pkg/codec/vpx/vpx_image.go
+++ /dev/null
@@ -1,40 +0,0 @@
-package vpx
-
-/*
-#cgo pkg-config: vpx
-#include <vpx/vpx_image.h>
-*/
-import "C"
-import "unsafe"
-
-type VpxImage struct {
-	img *C.vpx_image_t
-}
-
-func NewImageFromPtr(ptr *C.vpx_image_t) *VpxImage {
-	return &VpxImage{img: ptr}
-}
-
-func (i *VpxImage) Width() int {
-	return int(i.img.d_w)
-}
-
-func (i *VpxImage) Height() int {
-	return int(i.img.d_h)
-}
-
-func (i *VpxImage) YStride() int {
-	return int(i.img.stride[0])
-}
-
-func (i *VpxImage) UStride() int {
-	return int(i.img.stride[1])
-}
-
-func (i *VpxImage) VStride() int {
-	return int(i.img.stride[2])
-}
-
-func (i *VpxImage) Plane(n int) unsafe.Pointer {
-	return unsafe.Pointer(i.img.planes[n])
-}



Test patch:
diff --git a/pkg/codec/vpx/bitrate_tracker_test.go b/pkg/codec/vpx/bitrate_tracker_test.go
deleted file mode 100644
index 6a1da7d..0000000
--- a/pkg/codec/vpx/bitrate_tracker_test.go
+++ /dev/null
@@ -1,19 +0,0 @@
-package vpx
-
-import (
-	"math"
-	"testing"
-	"time"
-)
-
-func TestBitrateTracker(t *testing.T) {
-	packetSize := 1000
-	bt := NewBitrateTracker(time.Second)
-	bt.AddFrame(packetSize, time.Now())
-	bt.AddFrame(packetSize, time.Now().Add(time.Millisecond*100))
-	bt.AddFrame(packetSize, time.Now().Add(time.Millisecond*999))
-	eps := float64(packetSize*8) / 10
-	if got, want := bt.GetBitrate(), float64(packetSize*8)*3; math.Abs(got-want) > eps {
-		t.Fatalf("GetBitrate() = %v, want %v (|diff| <= %v)", got, want, eps)
-	}
-}
diff --git a/pkg/codec/vpx/vpx_image_test.go b/pkg/codec/vpx/vpx_image_test.go
deleted file mode 100644
index e54d3a1..0000000
--- a/pkg/codec/vpx/vpx_image_test.go
+++ /dev/null
@@ -1,94 +0,0 @@
-package vpx
-
-import (
-	"testing"
-	"unsafe"
-)
-
-// TestVpxImageStructure tests the VpxImage struct methods
-// Note: These tests verify the interface and structure without requiring actual VPX images
-func TestVpxImageStructure(t *testing.T) {
-	// Test that VpxImage can be created (interface test)
-	// We can't easily test with real C structures in unit tests due to CGO limitations
-	// but we can test the structure and interface
-	
-	t.Run("VpxImageInterface", func(t *testing.T) {
-		// This test ensures the VpxImage type exists and has the expected methods
-		// We use a type assertion to verify the interface
-		var _ interface {
-			Width() int
-			Height() int
-			YStride() int
-			UStride() int
-			VStride() int
-			Plane(int) unsafe.Pointer
-		} = (*VpxImage)(nil)
-	})
-}
-
-// TestNewImageFromPtr tests the constructor
-func TestNewImageFromPtr(t *testing.T) {
-	// Test with nil pointer
-	vpxImg := NewImageFromPtr(nil)
-	if vpxImg == nil {
-		t.Error("NewImageFromPtr should not return nil even with nil input")
-	}
-	if vpxImg != nil && vpxImg.img != nil {
-		t.Error("VpxImage should contain nil pointer when created with nil")
-	}
-}
-
-// TestVpxImageMethodsWithNil tests that methods panic appropriately with nil pointer
-// This documents the expected behavior - methods will panic if called with nil C pointer
-func TestVpxImageMethodsWithNil(t *testing.T) {
-	vpxImg := NewImageFromPtr(nil)
-	
-	// These methods should panic with nil img (this is expected behavior)
-	testCases := []struct {
-		name string
-		fn   func()
-	}{
-		{"Width", func() { vpxImg.Width() }},
-		{"Height", func() { vpxImg.Height() }},
-		{"YStride", func() { vpxImg.YStride() }},
-		{"UStride", func() { vpxImg.UStride() }},
-		{"VStride", func() { vpxImg.VStride() }},
-		{"Plane0", func() { vpxImg.Plane(0) }},
-		{"Plane1", func() { vpxImg.Plane(1) }},
-		{"Plane2", func() { vpxImg.Plane(2) }},
-	}
-	
-	for _, tc := range testCases {
-		t.Run(tc.name, func(t *testing.T) {
-			defer func() {
-				if r := recover(); r == nil {
-					t.Errorf("Method %s should panic with nil image but didn't", tc.name)
-				}
-			}()
-			tc.fn()
-		})
-	}
-}
-
-// TestVpxImageConstants tests expected behavior with common video formats
-func TestVpxImageConstants(t *testing.T) {
-	// Test that the VpxImage type can be used in common video processing scenarios
-	testCases := []struct {
-		name        string
-		planeIndex  int
-		description string
-	}{
-		{"Y Plane", 0, "Luma plane"},
-		{"U Plane", 1, "Chroma U plane"},
-		{"V Plane", 2, "Chroma V plane"},
-	}
-	
-	for _, tc := range testCases {
-		t.Run(tc.name, func(t *testing.T) {
-			// Verify plane indices are within expected range
-			if tc.planeIndex < 0 || tc.planeIndex > 2 {
-				t.Errorf("Plane index %d is out of expected range [0-2]", tc.planeIndex)
-			}
-		})
-	}
-}