wang-bin
commented
5 months ago can you try binaries from github actions to find out which commit breaks?
Open Vizalkar opened 5 months ago
wang-bin
commented
5 months ago can you try binaries from github actions to find out which commit breaks?
Vizalkar
commented
5 months ago Framework at commit [1ee41322] ensure all packet fifo set same max when buffering does produce the bug, while the framework at the previous commit [200e33f1] mf: fix early shutdown and crash in sample release does not. Tested on M1 Macbook Air with 20 videos in the exported scene, video used was ashthorp.mp4. Decoder used is vanilla FFmpeg
Not all videos get stuck so i guess this is a concurrency issue. I verified multiple times that [200e33f1] mf: fix early shutdown and crash in sample release did pass the test succesfully, while the other commit fail everytime and on several videos. I guess you cannot be 100% sure that the bug itself is effectively in [1ee41322] ensure all packet fifo set same max when buffering, it could also be simple correlation but if it is, the correlation is really strong
Regards Adrien
wang-bin
commented
5 months ago I expect that commit can fix the frozen but not introduce the frozen. I need your test program to debug, can you send me a binary?
Vizalkar
commented
5 months ago I sent you a link via email
Vizalkar
commented
5 months ago Are you able to reproduce the bug ? Do you have time available to work on this ? I'm asking because we need visibility to take a decision whether we release our new version with or without the new framework.
Regards
wang-bin
commented
5 months ago Yes. Working on it.
wang-bin
commented
5 months ago in heavym exported video all frames are the same. in boxes example + the latest sdk, i keep a few video items, frames after seek are correct. I also tested offlineRenderingStepForward code from glfwplay.cpp you sent me 2 years ago, no such issue.
wang-bin
commented
5 months ago renderVideo() return value is correct in boxes example, what about heavym?
Vizalkar
commented
5 months ago renderVideo() return value is correct in boxes example, what about heavym?
It is not correct in HeavyM no. NB : doc says renderVideo returns timestamp in microseconds but it seems that it's actually seconds, i tested that assertion in boxes.
The following is HeavyM code, logs below show that renderVideo always return the same timestamp, each one is stuck somewhere between 5000 ms and 7000 ms. Printed timestamps are in miliseconds.
void MDKPlayer::render()
{
/* [.........] */
const auto renderedPos = m_mdkPlayer->renderVideo(this) * 1000.f;
qDebug() << this << " renderedPos = " << QString::number(renderedPos)
<< " currentPosition = " << QString::number(m_currentPosition)
<< " difference = " << QString::number(m_currentPosition - renderedPos);
/* [.........] */
}
bool MDKWrapper::incrementTimeAndFrame(const double elapsed_msec)
{
/* [.......] */
m_currentPosition += elapsed_msec * m_speed;
/* [.......] */
qDebug() << this << " seek " << QString::number(m_currentPosition);
m_mdkPlayer->seek(
m_currentPosition,
mdk::SeekFlag::FromStart,
[doneSeekingCond, doneSeekingMutex, doneSeeking](auto)
{
std::unique_lock lock(*doneSeekingMutex);
*doneSeeking = true;
doneSeekingCond->notify_one();
});
constexpr int waitingTime_sec(10);
std::unique_lock doneSeekingLock(*doneSeekingMutex);
doneSeekingCond->wait_for(doneSeekingLock, std::chrono::seconds(waitingTime_sec), [doneSeeking]{ return *doneSeeking; });
if (!doneSeeking) qWarning() << "Failed to seek " + QString::fromStdString(m_mediaPath) + " (MDKWrapper::incrementTimeAndFrame)";
m_newFrameReadyCond.wait_for(newFrameReadyLock, std::chrono::seconds(waitingTime_sec), [this]{ return m_hasNewFrameReady.operator bool(); });
if (!m_hasNewFrameReady.operator bool()) qWarning() << "No frame ready in" + QString::fromStdString(m_mediaPath) + " (MDKWrapper::incrementTimeAndFrame)";
}NB : doc says renderVideo returns timestamp in microseconds but it seems that it's actually seconds, i tested that assertion in boxes.
unit is second, precision is 1 microsecond
what about seek callback position value?
wang-bin
commented
5 months ago btw, why do you seek for every frame?
Vizalkar
commented
5 months ago unit is second, precision is 1 microsecond
Thanks for the clarification
what about seek callback position value?
The seek callback value is correct
btw, why do you seek for every frame?
I need the video to be at the correct timestamp for each frame of the export. If the framerate of the video is less than the framerate of my exported video, i expect the seek function to handle the useless seek calls internally (as it is already the case i presume ?)
wang-bin
commented
5 months ago what about seek callback position value?
The seek callback value is correct
It's weird. I keep only 1 video in heavym, same result. but in boxes example renderVideo() result is correct.
btw, why do you seek for every frame?
I need the video to be at the correct timestamp for each frame of the export. If the framerate of the video is less than the framerate of my exported video, i expect the seek function to handle the useless seek calls internally (as it is already the case i presume ?)
no, it will seek to the same position multiple times. Maybe you can use Player.onSync() to sync all players:
Vizalkar
commented
5 months ago what about seek callback position value?
The seek callback value is correct
It's weird. I keep only 1 video in heavym, same result. but in boxes example renderVideo() result is correct.
Yes i know that is, previous frameworks make only a few video freeze (same code), but the recent frameworks make the bug more likely to happen (to the point it always happen). The bug could have been there for a long time in mdk and maybe the commit i pointed out is not the cause of the regression. I understand that it must be kind of uneasy to debug. Can you put a break point in renderVideo and in seek during the export to see what is going on ? I will keep trying to create circumstances where the bug shows in boxes as well.
no, it will seek to the same position multiple times. Maybe you can use
Player.onSync()to sync all players:
- for each player: player.onSync([timeline]{ return timeline.time();}); call once, set playing state.
- timeline.advance(); // increase 1/export_fps
- in renderer: for each player: while (timeline.time() - player.renderVideo() > 1/video_fps) { sleep a while}
- notify timeline to advance
No I cannot, it seems simple for a scenario where the timeline only plays videos from start to finish but in reality the timeline can be setup in many ways, with cues to trigger sequences, videos can be restarted throughout etc... Changing the method will impact so much code that it's going to be a huge mess. Moreover the advantage of this method is that the renderer has the same code than inline rendering, it's just the way we ask for a new frame that changes (one is realtime, the other one is done step by step)
Vizalkar
commented
5 months ago no, it will seek to the same position multiple times. Maybe you can use
Player.onSync()to sync all players:
- for each player: player.onSync([timeline]{ return timeline.time();}); call once, set playing state.
- timeline.advance(); // increase 1/export_fps
- in renderer: for each player: while (timeline.time() - player.renderVideo() > 1/video_fps) { sleep a while}
- notify timeline to advance
I tried the method you suggested on a simple case to see if results were promising or not and it was not very successfull anyway : videos would play at non constant rate in export, sometime speeding up or slowing down.
Have you been able to find why renderVideo and seek give a different timestamp result ? How can I help you to solve the seeking issue ?
Regards Adrien
wang-bin
commented
4 months ago Do you have arm64 heavym? It's too slow to open on my mac
Vizalkar
commented
4 months ago Do you have arm64 heavym? It's too slow to open on my mac
Unfortunately we don't, we need to switch to Qt 6 to compile an arm64 version and we have not yet gone through this work as this involves many breaking changes. I apologize
Vizalkar
commented
4 months ago Hi Could not reproduce the issue on version 0.29, have you solved the bug ?
wang-bin
commented
4 months ago no
Hi
In our software, we have a video export functionnality that allows the user to export its projection mapping in the form of a video. In this case we have an offline version of our renderer (as opposed to realtime) and we pause videos in the scene and seek the correct timestamp for every frame. (seek incremented timestamp -> wait for seeking done and new frame ready callback -> render frame -> loop) It seems that the latest MDK version does not produce the image associated with timestamp accurately anymore and videos appear to be frozen when seeked incremently in paused state.
Does that ring a bell on your side ? Could this commit be the cause of this ? [ecbd37db] ffmpeg: skip read when stream paused
Regards Adrien