[Scummvm-git-logs] scummvm master -> dcd537337be8b5e478415e4ce62fdaeba4ae18f5
bluegr
bluegr at gmail.com
Fri Nov 12 16:47:28 UTC 2021
This automated email contains information about 3 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .
Summary:
946bb818ea VIDEO: QuickTime comments, mild cleanup
6e3403464b VIDEO: Limit QuickTime workaround to Riven
dcd537337b VIDEO: Fix QuickTime decoding of edits with mediaTime
Commit: 946bb818eac9c62d4e46bfe50801fe643a17f058
https://github.com/scummvm/scummvm/commit/946bb818eac9c62d4e46bfe50801fe643a17f058
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2021-11-12T18:47:22+02:00
Commit Message:
VIDEO: QuickTime comments, mild cleanup
Changed paths:
common/quicktime.cpp
common/quicktime.h
video/qt_decoder.cpp
video/qt_decoder.h
diff --git a/common/quicktime.cpp b/common/quicktime.cpp
index 7b84b3aaea..3bbf4590b7 100644
--- a/common/quicktime.cpp
+++ b/common/quicktime.cpp
@@ -368,7 +368,6 @@ int QuickTimeParser::readTRAK(Atom atom) {
Track *track = new Track();
track->codecType = CODEC_TYPE_MOV_OTHER;
- track->startTime = 0; // XXX: check
_tracks.push_back(track);
return readDefault(atom);
@@ -837,7 +836,6 @@ QuickTimeParser::Track::Track() {
codecType = CODEC_TYPE_MOV_OTHER;
frameCount = 0;
duration = 0;
- startTime = 0;
mediaDuration = 0;
}
diff --git a/common/quicktime.h b/common/quicktime.h
index 9d91027043..52932f3cb6 100644
--- a/common/quicktime.h
+++ b/common/quicktime.h
@@ -93,7 +93,7 @@ protected:
struct TimeToSampleEntry {
int count;
- int duration;
+ int duration; // media time
};
struct SampleToChunkEntry {
@@ -103,9 +103,9 @@ protected:
};
struct EditListEntry {
- uint32 trackDuration;
- uint32 timeOffset;
- int32 mediaTime;
+ uint32 trackDuration; // movie time
+ uint32 timeOffset; // movie time
+ int32 mediaTime; // media time
Rational mediaRate;
};
@@ -148,7 +148,7 @@ protected:
uint32 *sampleSizes;
uint32 keyframeCount;
uint32 *keyframes;
- int32 timeScale;
+ int32 timeScale; // media time
uint16 width;
uint16 height;
@@ -158,18 +158,17 @@ protected:
Common::Array<EditListEntry> editList;
- uint32 frameCount;
- uint32 duration;
- uint32 mediaDuration;
- uint32 startTime;
+ uint32 frameCount; // from stts
+ uint32 duration; // movie time
+ uint32 mediaDuration; // media time
Rational scaleFactorX;
Rational scaleFactorY;
};
virtual SampleDesc *readSampleDesc(Track *track, uint32 format, uint32 descSize) = 0;
- uint32 _timeScale;
- uint32 _duration;
+ uint32 _timeScale; // movie time
+ uint32 _duration; // movie time
Rational _scaleFactorX;
Rational _scaleFactorY;
Array<Track *> _tracks;
diff --git a/video/qt_decoder.cpp b/video/qt_decoder.cpp
index 833873314d..bfa2e68680 100644
--- a/video/qt_decoder.cpp
+++ b/video/qt_decoder.cpp
@@ -294,7 +294,7 @@ QuickTimeDecoder::VideoTrackHandler::VideoTrackHandler(QuickTimeDecoder *decoder
checkEditListBounds();
_curEdit = 0;
- enterNewEditList(false);
+ enterNewEditListEntry(false);
_curFrame = -1;
_durationOverride = -1;
@@ -379,12 +379,12 @@ bool QuickTimeDecoder::VideoTrackHandler::seek(const Audio::Timestamp &requested
_curEdit++;
if (!atLastEdit())
- enterNewEditList(true);
+ enterNewEditListEntry(true);
return true;
}
- enterNewEditList(false);
+ enterNewEditListEntry(false);
// One extra check for the end of a track
if (atLastEdit()) {
@@ -402,7 +402,7 @@ bool QuickTimeDecoder::VideoTrackHandler::seek(const Audio::Timestamp &requested
_nextFrameStartTime += _durationOverride;
_durationOverride = -1;
} else {
- _nextFrameStartTime += getFrameDuration();
+ _nextFrameStartTime += getCurFrameDuration();
}
}
@@ -504,7 +504,7 @@ const Graphics::Surface *QuickTimeDecoder::VideoTrackHandler::decodeNextFrame()
if (atLastEdit())
return 0;
- enterNewEditList(true);
+ enterNewEditListEntry(true);
}
const Graphics::Surface *frame = bufferNextFrame();
@@ -516,7 +516,7 @@ const Graphics::Surface *QuickTimeDecoder::VideoTrackHandler::decodeNextFrame()
_durationOverride = -1;
} else {
// Just need to subtract the time
- _nextFrameStartTime -= getFrameDuration();
+ _nextFrameStartTime -= getCurFrameDuration();
}
} else {
if (_durationOverride >= 0) {
@@ -524,7 +524,7 @@ const Graphics::Surface *QuickTimeDecoder::VideoTrackHandler::decodeNextFrame()
_nextFrameStartTime += _durationOverride;
_durationOverride = -1;
} else {
- _nextFrameStartTime += getFrameDuration();
+ _nextFrameStartTime += getCurFrameDuration();
}
}
@@ -664,7 +664,7 @@ Common::SeekableReadStream *QuickTimeDecoder::VideoTrackHandler::getNextFramePac
return stream->readStream(_parent->sampleSizes[_curFrame]);
}
-uint32 QuickTimeDecoder::VideoTrackHandler::getFrameDuration() {
+uint32 QuickTimeDecoder::VideoTrackHandler::getCurFrameDuration() {
uint32 curFrameIndex = 0;
for (int32 i = 0; i < _parent->timeToSampleCount; i++) {
curFrameIndex += _parent->timeToSample[i].count;
@@ -688,7 +688,7 @@ uint32 QuickTimeDecoder::VideoTrackHandler::findKeyFrame(uint32 frame) const {
return frame;
}
-void QuickTimeDecoder::VideoTrackHandler::enterNewEditList(bool bufferFrames) {
+void QuickTimeDecoder::VideoTrackHandler::enterNewEditListEntry(bool bufferFrames) {
// Bypass all empty edit lists first
while (!atLastEdit() && _parent->editList[_curEdit].mediaTime == -1)
_curEdit++;
@@ -811,7 +811,7 @@ uint32 QuickTimeDecoder::VideoTrackHandler::getCurEditTimeOffset() const {
}
uint32 QuickTimeDecoder::VideoTrackHandler::getCurEditTrackDuration() const {
- // Need to convert to the track scale
+ // convert from movie time scale to the track's media time scale
return _parent->editList[_curEdit].trackDuration * _parent->timeScale / _decoder->_timeScale;
}
diff --git a/video/qt_decoder.h b/video/qt_decoder.h
index 3b1e461b84..8f90327bda 100644
--- a/video/qt_decoder.h
+++ b/video/qt_decoder.h
@@ -134,7 +134,7 @@ private:
Graphics::PixelFormat getPixelFormat() const;
int getCurFrame() const { return _curFrame; }
int getFrameCount() const;
- uint32 getNextFrameStartTime() const;
+ uint32 getNextFrameStartTime() const; // milliseconds
const Graphics::Surface *decodeNextFrame();
const byte *getPalette() const;
bool hasDirtyPalette() const { return _curPalette; }
@@ -151,9 +151,9 @@ private:
Common::QuickTimeParser::Track *_parent;
uint32 _curEdit;
int32 _curFrame;
- uint32 _nextFrameStartTime;
+ uint32 _nextFrameStartTime; // media time
Graphics::Surface *_scaledSurface;
- int32 _durationOverride;
+ int32 _durationOverride; // media time
const byte *_curPalette;
mutable bool _dirtyPalette;
bool _reversed;
@@ -165,13 +165,13 @@ private:
const Graphics::Surface *forceDither(const Graphics::Surface &frame);
Common::SeekableReadStream *getNextFramePacket(uint32 &descId);
- uint32 getFrameDuration();
+ uint32 getCurFrameDuration(); // media time
uint32 findKeyFrame(uint32 frame) const;
- void enterNewEditList(bool bufferFrames);
+ void enterNewEditListEntry(bool bufferFrames);
const Graphics::Surface *bufferNextFrame();
- uint32 getRateAdjustedFrameTime() const;
- uint32 getCurEditTimeOffset() const;
- uint32 getCurEditTrackDuration() const;
+ uint32 getRateAdjustedFrameTime() const; // media time
+ uint32 getCurEditTimeOffset() const; // media time
+ uint32 getCurEditTrackDuration() const; // media time
bool atLastEdit() const;
bool endOfCurEdit() const;
void checkEditListBounds();
Commit: 6e3403464b0bc09e06f94dc17be3f1df1d62b5a2
https://github.com/scummvm/scummvm/commit/6e3403464b0bc09e06f94dc17be3f1df1d62b5a2
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2021-11-12T18:47:22+02:00
Commit Message:
VIDEO: Limit QuickTime workaround to Riven
QuickTimeDecoder has a workaround for a video in a Spanish version of
Riven, but this workaround breaks valid QuickTime videos such as the
KQ6 Macintosh opening movie. (Bug #11085)
Until the original Riven video bug can be debugged to improve the
workaround, it is now disabled unless an engine enables it.
Workaround added in: b8abe400850a23d12fe5cdc24d7106820d0f13fd
Changed paths:
engines/mohawk/riven_video.cpp
video/qt_decoder.cpp
video/qt_decoder.h
diff --git a/engines/mohawk/riven_video.cpp b/engines/mohawk/riven_video.cpp
index 38d7e7f6c8..b0d20f3194 100644
--- a/engines/mohawk/riven_video.cpp
+++ b/engines/mohawk/riven_video.cpp
@@ -65,6 +65,7 @@ void RivenVideo::load(uint16 id) {
_video->setSoundType(Audio::Mixer::kSFXSoundType);
_video->setChunkBeginOffset(_vm->getResourceOffset(ID_TMOV, id));
_video->loadStream(_vm->getResource(ID_TMOV, id));
+ _video->enableEditListBoundsCheckQuirk(true); // for Spanish olw.mov
}
void RivenVideo::close() {
diff --git a/video/qt_decoder.cpp b/video/qt_decoder.cpp
index bfa2e68680..48a2a1c85a 100644
--- a/video/qt_decoder.cpp
+++ b/video/qt_decoder.cpp
@@ -51,6 +51,7 @@ namespace Video {
QuickTimeDecoder::QuickTimeDecoder() {
_scaledSurface = 0;
_width = _height = 0;
+ _enableEditListBoundsCheckQuirk = false;
}
QuickTimeDecoder::~QuickTimeDecoder() {
@@ -291,7 +292,9 @@ Audio::SeekableAudioStream *QuickTimeDecoder::AudioTrackHandler::getSeekableAudi
}
QuickTimeDecoder::VideoTrackHandler::VideoTrackHandler(QuickTimeDecoder *decoder, Common::QuickTimeParser::Track *parent) : _decoder(decoder), _parent(parent) {
- checkEditListBounds();
+ if (decoder->_enableEditListBoundsCheckQuirk) {
+ checkEditListBounds();
+ }
_curEdit = 0;
enterNewEditListEntry(false);
@@ -307,6 +310,12 @@ QuickTimeDecoder::VideoTrackHandler::VideoTrackHandler(QuickTimeDecoder *decoder
_ditherFrame = 0;
}
+// FIXME: This check breaks valid QuickTime movies, such as the KQ6 Mac opening.
+// It doesn't take media rate into account and mixes up units that are in movie
+// time scale and media time scale, which is easy to do since they're often the
+// same value. Other decoder bugs have been fixed since this was written, so it
+// would be good to re-evaluate what the problem was with the Riven Spanish video.
+// It's now disabled for everything except Riven.
void QuickTimeDecoder::VideoTrackHandler::checkEditListBounds() {
// Check all the edit list entries are within the bounds of the media
// In the Spanish version of Riven, the last edit of the video ogk.mov
diff --git a/video/qt_decoder.h b/video/qt_decoder.h
index 8f90327bda..c2aef7da3c 100644
--- a/video/qt_decoder.h
+++ b/video/qt_decoder.h
@@ -71,6 +71,8 @@ public:
const Graphics::Surface *decodeNextFrame();
Audio::Timestamp getDuration() const { return Audio::Timestamp(0, _duration, _timeScale); }
+ void enableEditListBoundsCheckQuirk(bool enable) { _enableEditListBoundsCheckQuirk = enable; }
+
protected:
Common::QuickTimeParser::SampleDesc *readSampleDesc(Common::QuickTimeParser::Track *track, uint32 format, uint32 descSize);
@@ -85,6 +87,8 @@ private:
void scaleSurface(const Graphics::Surface *src, Graphics::Surface *dst,
const Common::Rational &scaleFactorX, const Common::Rational &scaleFactorY);
+ bool _enableEditListBoundsCheckQuirk;
+
class VideoSampleDesc : public Common::QuickTimeParser::SampleDesc {
public:
VideoSampleDesc(Common::QuickTimeParser::Track *parentTrack, uint32 codecTag);
Commit: dcd537337be8b5e478415e4ce62fdaeba4ae18f5
https://github.com/scummvm/scummvm/commit/dcd537337be8b5e478415e4ce62fdaeba4ae18f5
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2021-11-12T18:47:22+02:00
Commit Message:
VIDEO: Fix QuickTime decoding of edits with mediaTime
QuickTimeDecoder has a bug which causes the mediaTime offset to be
ignored when a track begins with an empty edit and is followed by an
edit with a non-zero mediaTime. This causes the KQ6 Mac opening movie
to start several tracks at unintended frames (they're never supposed to
be displayed) and the intended frames at the end of the edit to never
be displayed. (Bug #11085)
Changed paths:
video/qt_decoder.cpp
video/qt_decoder.h
diff --git a/video/qt_decoder.cpp b/video/qt_decoder.cpp
index 48a2a1c85a..e0c5662803 100644
--- a/video/qt_decoder.cpp
+++ b/video/qt_decoder.cpp
@@ -297,9 +297,9 @@ QuickTimeDecoder::VideoTrackHandler::VideoTrackHandler(QuickTimeDecoder *decoder
}
_curEdit = 0;
- enterNewEditListEntry(false);
-
_curFrame = -1;
+ enterNewEditListEntry(true); // might set _curFrame
+
_durationOverride = -1;
_scaledSurface = 0;
_curPalette = 0;
@@ -514,6 +514,9 @@ const Graphics::Surface *QuickTimeDecoder::VideoTrackHandler::decodeNextFrame()
return 0;
enterNewEditListEntry(true);
+
+ if (isEmptyEdit())
+ return 0;
}
const Graphics::Surface *frame = bufferNextFrame();
@@ -697,14 +700,22 @@ uint32 QuickTimeDecoder::VideoTrackHandler::findKeyFrame(uint32 frame) const {
return frame;
}
-void QuickTimeDecoder::VideoTrackHandler::enterNewEditListEntry(bool bufferFrames) {
- // Bypass all empty edit lists first
- while (!atLastEdit() && _parent->editList[_curEdit].mediaTime == -1)
- _curEdit++;
+bool QuickTimeDecoder::VideoTrackHandler::isEmptyEdit() const {
+ return (_parent->editList[_curEdit].mediaTime == -1);
+}
+void QuickTimeDecoder::VideoTrackHandler::enterNewEditListEntry(bool bufferFrames) {
if (atLastEdit())
return;
+ // if this is an empty edit then the only thing to do is set the
+ // time for the next frame, which is the duration of this edit.
+ if (isEmptyEdit()) {
+ _curFrame = -1;
+ _nextFrameStartTime = getCurEditTimeOffset() + getCurEditTrackDuration();
+ return;
+ }
+
uint32 mediaTime = _parent->editList[_curEdit].mediaTime;
uint32 frameNum = 0;
uint32 totalDuration = 0;
@@ -792,8 +803,12 @@ const Graphics::Surface *QuickTimeDecoder::VideoTrackHandler::bufferNextFrame()
}
uint32 QuickTimeDecoder::VideoTrackHandler::getRateAdjustedFrameTime() const {
- // Figure out what time the next frame is at taking the edit list rate into account
- Common::Rational offsetFromEdit = Common::Rational(_nextFrameStartTime - getCurEditTimeOffset()) / _parent->editList[_curEdit].mediaRate;
+ // Figure out what time the next frame is at taking the edit list rate into account,
+ // unless this is an empty edit, in which case the rate isn't applicable.
+ Common::Rational offsetFromEdit = Common::Rational(_nextFrameStartTime - getCurEditTimeOffset());
+ if (!isEmptyEdit()) {
+ offsetFromEdit /= _parent->editList[_curEdit].mediaRate;
+ }
uint32 convertedTime = offsetFromEdit.toInt();
if ((offsetFromEdit.getNumerator() % offsetFromEdit.getDenominator()) > (offsetFromEdit.getDenominator() / 2))
diff --git a/video/qt_decoder.h b/video/qt_decoder.h
index c2aef7da3c..5f1a19cfcb 100644
--- a/video/qt_decoder.h
+++ b/video/qt_decoder.h
@@ -171,6 +171,7 @@ private:
Common::SeekableReadStream *getNextFramePacket(uint32 &descId);
uint32 getCurFrameDuration(); // media time
uint32 findKeyFrame(uint32 frame) const;
+ bool isEmptyEdit() const;
void enterNewEditListEntry(bool bufferFrames);
const Graphics::Surface *bufferNextFrame();
uint32 getRateAdjustedFrameTime() const; // media time
More information about the Scummvm-git-logs
mailing list