fix _get_frames() for PyAV version 10

[BlGene]

The notebook _get_frames function was broken for PyAV 10 (the requirements.txt file lists PyAV 8).
To have something compatible with both av version, a specific _get_frames function could be selected based on av.__version__.

Note: this is a partial replacement as it ignores audio.

[facebook-github-bot]

Hi @BlGene!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

[miguelmartin75]

Hi, thanks for your contribution!

You should be aware, your code may not handle all cases for the Ego4D videos. Specifically you need to take into account for the video stream's start time.

What is the reason for it not working with more recent PyAV versions?

I think a more robust solution would be to modify the seek's performed in the code (assuming this is the reason for it not working for more recent versions). You can seek on the container using the video or audio stream's timebase: https://pyav.org/docs/stable/api/container.html#av.container.InputContainer.seek, i.e. container.seek(pts, stream=video_stream, ...)

[BlGene]

Hi Miguel,

The reason for it not working with the new pyav version is that with the _get_frames function, the variable video_stream is of type av.video.codeccontext.VideoCodecContext , which does not provide a seek method. See the error below. As you suggest, I'm now doing the seek on the container.

Yes, it looks like switching to container.seek(seek_pts, stream=video_stream) would work, because it would then use the video stream time base, which is also done in the calculation of seek_pts. This would be the minimal fix and might thus be the best solution.

Some more notes:

1. Out of curiosity, I don't quite understand what additional account needs to be taken for the video stream's start time, the seek depends only on frame numbers (which I assume always start at 0) and the frame rate.
2. I haven't tested a lot, but I haven't had issues with not precise seeks, this may be alleviated by use of microsecond time base?
3. The new function should be a bit quicker as it does a seek in the loop over video_frames.

Explanation of seek time bases:

This is just a bit more explanation based on how I got to my solution, that hopefully clarifies the different time bases used for the seek function. I tried to seek to the old seek_pts value, however this was taking quite long, and printing in the for frame in container.decode(**streams_to_decod.. loop showed that it was iterating thought all frames from the beginning. I found some code here: PyAV-Org/PyAV#1113 and it seemed to work ok.

After your question, I did some more quick debugging, and it looks like there is also potentially a units problem occurring. I did the debugging using a simple call, looking for frame 1000. With _get_frames the seek target variable seek_pts had value 510976, whereas the _get_frames function has the seek target variable sec*1000000 has value 33000000. Looking at the documentation of the seek function says its input offset variable should be the "Time to seek to, expressed instream.time_base if stream is given, otherwise in av.time_base". In both cases, we are not giving a stream, so we should be giving the time in units of av.time_base which in my version of av is 1000000, so microseconds, consistent with the code snippet I choose. In case this worked correctly in the old version, it's probably because the seek was done on the video_stream and this implicitly used the video time_base.

Error Message

AttributeError                            Traceback (most recent call last)
Cell In[434], line 2
      1 with av.open(video_path) as input_video:
----> 2     frames = list(_get_frames([0,1000], input_video, include_audio=False, audio_buffer_frames=0))

File ~/lang/Ego4d/notebooks/nb_video_utils.py:62, in _get_frames_pts(video_pts_set, container, include_audio, include_additional_audio_pts)
     59 # seek to the point we need in the video
     60 # with some buffer room, just in-case the seek is not precise
     61 seek_pts = max(0, min_pts - 2 * video_pt_diff)
---> 62 video_stream.seek(seek_pts)
     63 if "audio" in streams_to_decode:
     64     assert len(container.streams.audio) == 1

File av/stream.pyx:112, in av.stream.Stream.__getattr__()

AttributeError: 'av.video.codeccontext.VideoCodecContext' object has no attribute 'seek'```