Add media duration to lpms_get_codec_info for GetCodecInfo

[eliteprox]

Adds the duration to lpms_get_codec_info for audio only and audio/video files.

This is needed for the audio to text pipeline

[leszko]

Thanks for the PR. Added 2 comments.

Other than that, @j0sh could you also look at the PR (since I believe you have the best knowledge about the lpms project)?

[j0sh]

This is good, it would be nice to also add checks for expected fps/duration values from ffprobe - helps ensure that our own calculations are correct

also just checking, which one of these triggers the no-duration case that requires manual calculations? (that should also surface via ffprobe, eg a duration of "N/A")

[eliteprox]

It seems to be with webm and mp3 files. ffprobe -show_streams test.webm | grep 'duration=' gives N/A for duration. I've added webm to the test, it is working with the latest changes.

[j0sh]

I took a look and the duration is there for the top-level container (ic->duration) for both webm and mp3 files. You can also see this in the top-level ffprobe output, or ffprobe -show_format. Stream duration is indeed empty (which is the equivalent with ffprobe -show_streams) but we aren't using that here.

If we need to introduce a code path to manually calculate the duration then I would prefer to find a test case which can exercise that.

Also please add the ffprobe outputs to the test case as well, because this validates the assumptions we are testing against, and allows us to spot areas where we diverge from ffmpeg's own behavior. For example:

ffmpeg -loglevel warning -i test.mp4 -c:v libvpx -c:a vorbis -strict -2 -t 1 test.webm
ffprobe -loglevel warning -show_format test.webm | grep duration=1.047000
ffprobe -loglevel warning -show_streams -select_streams v test.webm | grep r_frame_rate=24/1

this example is shortened to 1 second; it doesn't really need to be very long for the purpose of this test case and speeds up test execution since its encoding video which is expensive

BTW the durations you have in these test cases seem very round - do you care about the fractional part of the second at all? If not, then maybe make it an integer type instead of floating point. Otherwise you'll need to cast to double when calculating the duration, eg out->dur = (double)ic->duration / AV_TIME_BASE;

[eliteprox]

Added the ffprobe commands, shortened all the videos to 2 seconds. The int64 change is working well so I removed calculate_stream_duration. I am happy with rounding numbers for now

[j0sh]

I am happy with rounding numbers for now

Double checking (no pun intended) ... anything less than 1 (eg, 0.99) will be rounded down to 0, 1.99 becomes 1, etc. That seems pretty significant (eg, how do you distinguish between "very short" and "zero duration") but if is still OK for your purposes, then 👍

Otherwise, I believe we can still match the values calculated by ffprobe - our equivalent would be out->dur = ic->duration * av_q2d(AV_TIME_BASE_Q) ... mathematically the same as the old code, but I don't know if the computed precision would differ (and it would only come into play for very large durations, anyhow)

[j0sh]

I took a look and the duration is there for the top-level container (ic->duration) for both webm and mp3 files. You can also see this in the top-level ffprobe output, or ffprobe -show_format. Stream duration is indeed empty (which is the equivalent with ffprobe -show_streams) but we aren't using that here.

If we need to introduce a code path to manually calculate the duration then I would prefer to find a test case which can exercise that.

Also please add the ffprobe outputs to the test case as well, because this validates the assumptions we are testing against, and allows us to spot areas where we diverge from ffmpeg's own behavior. For example:

ffmpeg -loglevel warning -i test.mp4 -c:v libvpx -c:a vorbis -strict -2 -t 1 test.webm
ffprobe -loglevel warning -show_format test.webm | grep duration=1.047000
ffprobe -loglevel warning -show_streams -select_streams v test.webm | grep r_frame_rate=24/1

this example is shortened to 1 second; it doesn't really need to be very long for the purpose of this test case and speeds up test execution since its encoding video which is expensive

BTW the durations you have in these test cases seem very round - do you care about the fractional part of the second at all? If not, then maybe make it an integer type instead of floating point. Otherwise you'll need to cast to double when calculating the duration, eg out->dur = (double)ic->duration / AV_TIME_BASE;

[j0sh]

LGTM! 🚢

please squash when merging

[rickstaa]

@eliteprox very clean pull request 🚀. @j0sh thanks for helping to review this 🙏🏻.