Add fps and duration to GetCodecInfo

[ad-astra-video]

Add FPS and Duration data to GetCodecInfo.

[leszko]

Nice! I tested it and it worked fine for every video I tried! 🎉

Added two comments just for me to understand the changes.

[leszko]

What does this line do? Why isn't it just this?

out->fps = frame_rate.num;

[ad-astra-video]

frame rates are in AVRational types that are similar to Rats in Go with a numerator and denominator. I think this is overly conservative after researching a bit more and changing to avg_frame_rate in the AVStream

[leszko]

Just to make sure, did you check the performance of this call? Won't it slow down the input file analysis?

[ad-astra-video]

I have updated to use avg_frame_rate in the AVStream from the avformat_find_stream_info call already being done. Duration is in the AVFormatContext as well so don't need to calculate. Want to note that the nb_frames was not populated on webm files I think this is a container spec driven difference compared to mp4 files.

[ad-astra-video]

Performance stats indicate it performs more or less the same as not getting the FPS and Duration data. Was tested on my O node so not entirely unused test environment.

Most test files from https://test-videos.co.uk

perf_test.txt

test function was:

func main() {
	p := "/home/brad/test-videos"
	items, _ := os.ReadDir(p)
	w := tabwriter.NewWriter(os.Stdout, 1, 1, 1, ' ', tabwriter.Debug)

	fmt.Fprintln(w, "File\tTook (ms)\tFPS\tDur")
	for _, item := range items {
		vid_path := p + "/" + item.Name()
		//fmt.Println(item.Name())
		start := time.Now()
		_, mfi, err := ffmpeg.GetCodecInfo(vid_path)
		if err == nil {
			took := time.Since(start).Milliseconds()
			line := fmt.Sprintf("%v\t%v\t%v\t%v", item.Name(), took, mfi.FPS, mfi.Dur)
			fmt.Fprintln(w, line)
		} else {
			fmt.Printf("error getting codec info: %w\n", err)
		}
	}

	w.Flush()
}

[leszko]

Looks good. @ad-astra-video ping me when the PR is ready to merge, then I'll merge it.

[leszko]

@ad-astra-video Is this PR ready to merge?

[ad-astra-video]

I think the lpms part has no changes needed. go-livepeer does not consider getting the fps from the new fields yet though. Do you want to wait to merge until that gets added to go-livepeer?

[leszko]

I think the lpms part has no changes needed. go-livepeer does not consider getting the fps from the new fields yet though. Do you want to wait to merge until that gets added to go-livepeer?

Yeah, maybe let's wait and merge it all together

[ad-astra-video]

Rebased this to current master. Testing this with AI workflows.

[j0sh]

Is this PR still needed now that #407 is merged?