Skip to content

Conversation

@eloisebrosseau
Copy link
Contributor

Enable first frame to start at 0

Summarize your change.

If there is no timecode, the first frame will be 0 or the value calculated from the start_time field of the AVFormatContext. The offset is also adjusted to be at least 0 to prevent it from being negative.

Describe the reason for the change.

In RV, if the media has no timecode, the first frame was being set to 1 if the start_time field of the AVFormatContext is 0. However, this was preventing the frame to be set to 0 to match the timeline of the Review App. The offset was also being set to a negative value when the first frame was set to 0.

Describe what you have tested and on which operating system.

I manually tested a few movie files with different timecode and without any in RV with and without Live Review on MacOS.

Add a list of changes, and note any that might need special attention during the review.

static_cast<double>(m_avFormatContext->start_time) / static_cast<double>(AV_TIME_BASE)));
if (evStartFrameAtOne.getValue())
{
firstFrame = max(int64_t(m_formatStartFrame), int64_t(1));
Copy link
Contributor

@bernie-laberge bernie-laberge Nov 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct me if I am wrong but the m_formatStartFrame is no longer initialized right ? (with or without the new env var)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Is the variable still needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not needed anymore, but I did not update the code since I'm not sure we still need the changes from this PR. This was first done to try to fix an issue with Live Review in a web to RV session that has since been fixed from the web side. I kept this PR open in the meantime, but I think it might be fine to close it and let RV have its first frame at 1 since it is now able to set it to 0 when that's the value sent by the ReviewApp without changing anything.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we could close it and we can always re-open it if we realize we need it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants