[Bug]Sample presentantion timestamp

[brabebhin]

Hello,

I discovered a rather disturbing bug. Assume we have a standard ffmpeg interop MSS, and a MediaPlayer object as consumer. If the MediaPlayer object has AutoPlay false, and you try to seek programmatically before the first sample is delivered, the subsequent samples have the wrong presentation time.

[brabebhin]

The problem is located in MediaSampleProvider::GetNextPackage

if (SUCCEEDED(hr))
{
    // Write the packet out
    hr = WriteAVPacketToStream(writer, &avPacket);

    if (m_startOffset == -1)
    {
        //if we havent set m_startOffset already
        DebugMessage(L"Saving m_startOffset\n");

        //in some real-time streams framePts is less than 0 so we need to make sure m_startOffset is never negative
        m_startOffset = framePts < 0 ? 0 : framePts;
    }

    pts = LONGLONG(av_q2d(m_pAvFormatCtx->streams[m_streamIndex]->time_base) * 10000000 * (framePts - m_startOffset));

    dur = LONGLONG(av_q2d(m_pAvFormatCtx->streams[m_streamIndex]->time_base) * 10000000 * frameDuration);
}

m_startOffset will always cause samples to be presented by the beginning of the stream instead of their actual presentation time. This makes seeking before playing or MediaPlaybackItems with a start position other than 0 unusable. I am not sure what the purpose of m_startOffset is supposed to be. Maybe it needs to ignore streams where the starting offset is bigger than 0.

For example, if I seek to 5136629760 while m_startOffset is still -1, I will get a starting offset of 5136629760, which will make the sample at 5136629760 be presented at timestamp 0. Maybe m_startOffset needs to be set somewhere else.

[brabebhin]

@khouzam try

m_startOffset = m_pAvFormatCtx->streams[m_streamIndex]->start_time < 0 ? 0 : m_pAvFormatCtx->streams[m_streamIndex]->start_time;

[reego-fr]

Thanks @mcosmin222, I didn't think about this use case. How is it with https://github.com/Microsoft/FFmpegInterop/commit/02b3be1ef04a988130f91873763616513e9f4c84 ? As far as I am concerned this is the way to fix the original issue with start offsets.

[brabebhin]

I see. I will try. I never understood exactly what the issue was in that ticket. And I never encountered the need for weird files that would need that offset. As far as I am concerned, as long as the bugged code I presented above is avoided, it should be all good.

[reego-fr]

Just in case you didn't notice, https://github.com/Microsoft/FFmpegInterop/commit/94e6d58ff85323c5fdb334b140b5809c87e081db is the parent of https://github.com/Microsoft/FFmpegInterop/commit/02b3be1ef04a988130f91873763616513e9f4c84 and part of the fix. It is needed for some of the vob files found on DVDs or *.ts and other dumps of digital TV.

[brabebhin]

@reego-fr , your fix seems to be based on an older version. I will see to integrate it into newer version.

[brabebhin]

Seems good, but I think we should have one of these weird files stored somewhere for tests. We need nonregression tests for this

[khouzam]

Yes, please, the more test cases we can validate on changes the better this will be.

[brabebhin]

@khouzam , is there any ETA when this fix will be merged into the main repository?

[khouzam]

Sorry. I've been working on other projects. I can find some time this week for this.