video.py read_video_timestamps calculate pts without storing full frames

[mjunyent]

To calculate the PTS the function is storing the full frame objects on memory. This makes it crash for longer videos as it can't fill everything. PTS can be calculated directly.

With this fix we don't call _read_from_stream. That's ok as most of the code deals with seeking which is not needed in this case, but video decoding is surrounded by a try/except that we're not doing here. I'm not sure if a try/except av.AVError should be added too as it's not done in the demux either. Also, do we really want to mute this exception?

[fmassa]

Thanks a lot, this was an oversight I believe!

I do think that we had issues with AVError being raised for some cases though, so might be preferable to handle those exceptions in here as well.

My thinking is that if we have a corrupted file in the dataset, we should not break the training code right away but instead skip this file, which was handled in the previous implementation.

Let me know what you think

[mjunyent]

It makes sense. I don't have any problem in adding a check for AVError but then shouldn't we add one too when _can_read_timestamps_from_packets is True? Or _can_read_timestamps_from_packets ensures demux won't crash.

[fmassa]

Let's move forward with this as this is a net improvement I think.
But I think for a follow-up PR we should add another check for if the decoding fails, so that we can keep the behavior as before.

Can you send a follow-up PR?

[fmassa]

Thanks a lot!