refactor(ffmpeg): cleanup callbacks and increased coverage

ffmpeg/callback.go

@@ -47,19 +47,6 @@ func goPacketSeek(opaque unsafe.Pointer, offset C.int64_t, whence C.int) C.int64
 	return C.int64_t(n)
 }
 
-//export goInterrupt
-func goInterrupt(opaque unsafe.Pointer) C.int {
-	if ctx, ok := pointer.Restore(opaque).(*AVContext); ok {
-		select {
-		case <-ctx.context.Done():
-			return 1
-		default:
-			return 0
-		}
-	}
-	return 0
-}
-
 //export goAVLoggingHandler
 func goAVLoggingHandler(level C.int, cstr *C.char) {
 	log(AVLogLevel(level), C.GoString(cstr))

ffmpeg/ffmpeg.c

@@ -39,10 +39,6 @@ int create_format_context(AVFormatContext *fmt_ctx, void* opaque, int flags) {
         seeker = goPacketSeek;
         seekable = 1;
     }
-    if (flags & INTERRUPT_FLAG) {
-        fmt_ctx->interrupt_callback.callback = goInterrupt;
-        fmt_ctx->interrupt_callback.opaque = opaque;
-    }
     if (!(avio_ctx = avio_alloc_context(avio_buffer, BUFFER_SIZE, write_flag, opaque, reader, NULL, seeker))) {
         av_free(avio_buffer);
         avformat_free_context(fmt_ctx);

ffmpeg/ffmpeg.go

@@ -17,7 +17,6 @@ import (
 const (
 	readPacketFlag = 1
 	seekPacketFlag = 2
-	interruptFlag  = 3
 	hasVideo       = 1
 	hasAudio       = 2
 )
@@ -58,9 +57,8 @@ type AVContext struct {
 	closed             bool
 }
 
-func LoadAVContext(ctx context.Context, reader io.Reader, size int64) (*AVContext, error) {
+func LoadAVContext(reader io.Reader, size int64) (*AVContext, error) {
 	av := &AVContext{
-		context:       ctx,
 		reader:        reader,
 		size:          size,
 		selectedIndex: -1,
@@ -68,7 +66,7 @@ func LoadAVContext(ctx context.Context, reader io.Reader, size int64) (*AVContex
 	if seeker, ok := reader.(io.Seeker); ok {
 		av.seeker = seeker
 	}
-	flags := C.int(readPacketFlag | interruptFlag)
+	flags := C.int(readPacketFlag)
 	if av.seeker != nil {
 		flags |= seekPacketFlag
 	}
@@ -111,7 +109,7 @@ func (av *AVContext) Export(bands int) (buf []byte, err error) {
 		return
 	}
 	if bands < 3 || bands > 4 {
-		bands = 3
+		bands = 4
 	}
 	if err = convertFrameToRGB(av, bands); err != nil {
 		return

ffmpeg/ffmpeg.h

@@ -16,7 +16,6 @@
 #define BUFFER_SIZE 1 << 12
 #define READ_PACKET_FLAG 1
 #define SEEK_PACKET_FLAG 2
-#define INTERRUPT_FLAG 3
 #define HAS_VIDEO_STREAM 1
 #define HAS_AUDIO_STREAM 2
 #define ERR_TOO_BIG FFERRTAG('H','M','M','M')
@@ -70,5 +69,3 @@ void populate_histogram(ThumbContext *thumb_ctx, int n, AVFrame *frame);
 extern int goPacketRead(void *opaque, uint8_t *buf, int buf_size);
 
 extern int64_t goPacketSeek(void *opaque, int64_t seek, int whence);
-
-extern int goInterrupt(void *opaque);

ffmpeg/ffmpeg_test.go

@@ -1,13 +1,14 @@
 package ffmpeg
 
 import (
-	"context"
 	"encoding/json"
 	"fmt"
 	"github.com/cshum/imagor/vips"
+	"github.com/cshum/imagor/vips/pointer"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.uber.org/zap"
+	"io"
 	"os"
 	"reflect"
 	"strings"
@@ -53,29 +54,28 @@ func TestAVContext(t *testing.T) {
 	require.NoError(t, os.MkdirAll(baseDir+"golden/export", 0755))
 	t.Parallel()
 	for _, filename := range files {
-		for _, frame := range []int{-1, 5, 10, 9999, 99999} {
+		for _, n := range []int{-1, 5, 10, 9999, 99999} {
 			name := filename
-			if frame > -1 {
+			if n > -1 {
-				name = fmt.Sprintf("%s-%d", filename, frame)
+				name = fmt.Sprintf("%s-%d", filename, n)
 			}
 			t.Run(name, func(t *testing.T) {
-				ctx := context.Background()
 				path := baseDir + filename
 				reader, err := os.Open(path)
 				require.NoError(t, err)
 				stats, err := os.Stat(path)
 				require.NoError(t, err)
-				av, err := LoadAVContext(ctx, reader, stats.Size())
+				av, err := LoadAVContext(reader, stats.Size())
 				require.NoError(t, err)
 				defer av.Close()
-				if frame == 10 {
+				if n == 10 {
-					require.NoError(t, av.ProcessFrames(frame))
+					require.NoError(t, av.ProcessFrames(n))
 				} else {
-					if frame == 9999 {
+					if n == 9999 {
 						require.NoError(t, av.ProcessFrames(-1))
 					}
-					if frame > -1 {
+					if n > -1 {
-						require.NoError(t, av.SelectFrame(frame))
+						require.NoError(t, av.SelectFrame(n))
 					}
 				}
 				meta := av.Metadata()
@@ -88,8 +88,14 @@ func TestAVContext(t *testing.T) {
 					require.NoError(t, os.WriteFile(goldenFile, metaBuf, 0666))
 				}
 				bands := 4
+				if n == 99999 {
+					bands = 999
+				}
 				buf, err := av.Export(bands)
 				require.NoError(t, err)
+				if bands > 4 {
+					bands = 4
+				}
 				img, err := vips.LoadImageFromMemory(buf, meta.Width, meta.Height, bands)
 				require.NoError(t, err)
 				buf, err = img.ExportJpeg(nil)
@@ -109,29 +115,70 @@ func TestNoVideo(t *testing.T) {
 	require.NoError(t, os.MkdirAll(baseDir+"golden/meta", 0755))
 	require.NoError(t, os.MkdirAll(baseDir+"golden/export", 0755))
 	for _, filename := range noVideo {
-		t.Run(filename, func(t *testing.T) {
+		for i := 0; i < 2; i++ {
-			ctx := context.Background()
+			t.Run(fmt.Sprintf("%s-%d", filename, i), func(t *testing.T) {
-			path := baseDir + filename
+				path := baseDir + filename
-			reader, err := os.Open(path)
+				reader, err := os.Open(path)
-			require.NoError(t, err)
+				require.NoError(t, err)
-			stats, err := os.Stat(path)
+				stats, err := os.Stat(path)
-			require.NoError(t, err)
+				require.NoError(t, err)
-			av, err := LoadAVContext(ctx, reader, stats.Size())
+				av, err := LoadAVContext(reader, stats.Size())
-			require.NoError(t, err)
+				require.NoError(t, err)
-			defer av.Close()
+				defer av.Close()
-			require.Equal(t, ErrDecoderNotFound, av.ProcessFrames(-1))
+				require.Equal(t, ErrDecoderNotFound, av.ProcessFrames(-1))
-			meta := av.Metadata()
+				meta := av.Metadata()
-			metaBuf, err := json.Marshal(meta)
+				metaBuf, err := json.Marshal(meta)
-			require.NoError(t, err)
+				require.NoError(t, err)
-			goldenFile := baseDir + "golden/meta/" + filename + ".meta.json"
+				goldenFile := baseDir + "golden/meta/" + filename + ".meta.json"
-			if curr, err := os.ReadFile(goldenFile); err == nil {
+				if curr, err := os.ReadFile(goldenFile); err == nil {
-				assert.Equal(t, string(curr), string(metaBuf))
+					assert.Equal(t, string(curr), string(metaBuf))
-			} else {
+				} else {
-				require.NoError(t, os.WriteFile(goldenFile, metaBuf, 0666))
+					require.NoError(t, os.WriteFile(goldenFile, metaBuf, 0666))
-			}
+				}
-			buf, err := av.Export(3)
+				if i == 0 {
-			require.Empty(t, buf)
+					buf, err := av.Export(3)
-			require.Equal(t, ErrDecoderNotFound, err)
+					require.Empty(t, buf)
-		})
+					assert.Equal(t, ErrDecoderNotFound, err)
+				} else {
+					assert.Equal(t, ErrDecoderNotFound, av.SelectFrame(1))
+				}
+			})
+		}
 	}
 }
+
+func TestCorrupted(t *testing.T) {
+	filename := "macabre.mp4"
+	path := baseDir + filename
+	file, err := os.Open(path)
+	require.NoError(t, err)
+	reader := &readCloser{
+		Reader: io.LimitReader(file, 1024),
+		Closer: file,
+	}
+	stats, err := os.Stat(path)
+	require.NoError(t, err)
+	av, err := LoadAVContext(reader, stats.Size())
+	require.Equal(t, ErrInvalidData, err)
+	require.Empty(t, av)
+}
+
+func TestCorruptedOpaque(t *testing.T) {
+	filename := "macabre.mp4"
+	path := baseDir + filename
+	reader, err := os.Open(path)
+	require.NoError(t, err)
+	stats, err := os.Stat(path)
+	require.NoError(t, err)
+	av, err := LoadAVContext(reader, stats.Size())
+	require.NoError(t, err)
+	defer av.Close()
+	pointer.Unref(av.opaque)
+	err = av.ProcessFrames(-1)
+	assert.Equal(t, ErrUnknown, err)
+}
+
+type readCloser struct {
+	io.Reader
+	io.Closer
+}

processor.go

@@ -108,7 +108,7 @@ func (p *Processor) Process(ctx context.Context, in *imagor.Blob, params imagorp
 			return
 		}
 	}
-	av, err := ffmpeg.LoadAVContext(ctx, r, size)
+	av, err := ffmpeg.LoadAVContext(r, size)
 	if err != nil {
 		return
 	}