Skip to content

Commit 39565b9

Browse files
danogdanog
authored
Merge pull request #1938 from danog/fix-recorder-codec-switch
Recordings follow a codec change on the same video source
2 parents 90adce8 + 0276e30 commit 39565b9

7 files changed

Lines changed: 106 additions & 16 deletions

File tree

‎composer.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,12 @@
6666
"danog/tg-dialog-id": "^2",
6767
"symfony/polyfill-php83": "^1.41",
6868
"danog/php-rtc-av": "^1.1.11",
69-
"danog/php-rtc-codecs": "^1.1.19",
69+
"danog/php-rtc-codecs": "^1.1.20",
7070
"danog/php-rtc-dtls": "^2.0.19",
7171
"danog/php-rtc-ice": "^2.0.18",
7272
"danog/php-rtc-ntp": "^1.1.14",
7373
"danog/php-rtc-rtcp": "^1.1.11",
74-
"danog/php-rtc-rtp": "^2.0.24",
74+
"danog/php-rtc-rtp": "^2.0.25",
7575
"danog/php-rtc-sctp": "^2.0.14",
7676
"danog/php-rtc-srtp": "^1.1.16",
7777
"danog/php-rtc-stun": "^2.0.18",

‎src/API.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ final class API extends AbstractAPI
5454
*
5555
* @var string
5656
*/
57-
public const RELEASE = '8.15.0';
57+
public const RELEASE = '8.15.1';
5858
/**
5959
* We're not logged in.
6060
*

‎src/Tgcalls/CallRecorder.php‎

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -324,8 +324,11 @@ public function pushAudioFrame(string $data, int $rtpTimestamp, int $source = 0)
324324
* @param int $source Identifies the track the frame belongs to; a new source is placed on
325325
* the recording's clock where it currently is.
326326
* @param int $slot Whether this is the camera ({@see CallStream::VIDEO}) or the screen share ({@see CallStream::SCREEN}).
327+
* @param ?string $codecId The Matroska codec ID of the frame, if known (from the payload type it arrived with,
328+
* see {@see self::codecIdOf()}): a sender can switch codec on the same source, which
329+
* is then described afresh, like a new source. Sniffed from the bitstream otherwise.
327330
*/
328-
public function pushVideoFrame(string $data, int $rtpTimestamp, int $source = 0, int $slot = CallStream::VIDEO): void
331+
public function pushVideoFrame(string $data, int $rtpTimestamp, int $source = 0, int $slot = CallStream::VIDEO, ?string $codecId = null): void
329332
{
330333
if ($this->closed || $data === '') {
331334
return;
@@ -334,10 +337,11 @@ public function pushVideoFrame(string $data, int $rtpTimestamp, int $source = 0,
334337
if ($this->expected[$slot] === false || !$this->wanted($slot)) {
335338
return; // Signaling says this stream is off (a straggler), or it is not recorded.
336339
}
337-
$newSource = ($this->sources[$slot] ?? null) !== $source;
340+
$newSource = ($this->sources[$slot] ?? null) !== $source
341+
|| ($codecId !== null && isset($this->described[$slot]) && $this->described[$slot]['codecId'] !== $codecId);
338342
$ms = $this->placeOnClock($slot, $source, $rtpTimestamp, 90000);
339343

340-
[$out, $keyframe] = self::transformVideo($data, $newSource ? null : ($this->described[$slot]['codecId'] ?? null));
344+
[$out, $keyframe] = self::transformVideo($data, $codecId ?? ($newSource ? null : ($this->described[$slot]['codecId'] ?? null)));
341345
if ($out === '') {
342346
return;
343347
}
@@ -790,6 +794,24 @@ public static function describeVideo(string $frame): array
790794
return ['V_VP8', 1280, 720, ''];
791795
}
792796

797+
/**
798+
* The Matroska codec ID of a received frame's codec, from the MIME type of its payload type
799+
* ({@see \Webrtc\Codecs\EncodedPacket::getMimeType()}), if it is a video codec we record.
800+
*
801+
* @psalm-pure
802+
*/
803+
public static function codecIdOf(?string $mimeType): ?string
804+
{
805+
return match ($mimeType === null ? null : strtolower($mimeType)) {
806+
'video/vp8' => 'V_VP8',
807+
'video/vp9' => 'V_VP9',
808+
'video/av1' => 'V_AV1',
809+
'video/h264' => 'V_MPEG4/ISO/AVC',
810+
'video/h265' => 'V_MPEGH/ISO/HEVC',
811+
default => null,
812+
};
813+
}
814+
793815
/**
794816
* Prepare a frame for muxing and report whether it is a keyframe.
795817
*

‎src/Tgcalls/Controller.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -558,7 +558,7 @@ private function drainIncomingVideo(int $index): void
558558
$this->incomingVideoTracks[$index]['target'] = $target;
559559
$this->call->log("The incoming video track $source of {$this->call} is the peer's ".CallStream::NAMES[$target], Logger::VERBOSE);
560560
}
561-
$this->media->pushVideoFrame($data, $frame->getTimestamp(), $source, $target);
561+
$this->media->pushVideoFrame($data, $frame->getTimestamp(), $source, $target, CallRecorder::codecIdOf($frame->getMimeType()));
562562
}
563563
});
564564
}

‎src/Tgcalls/IncomingMedia.php‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -441,7 +441,7 @@ private function drainVideo(string $key): void
441441
return;
442442
}
443443
if ($frame instanceof EncodedPacket) {
444-
$this->pushVideoFrame($frame->getData(), $frame->getTimestamp(), $source, $slot);
444+
$this->pushVideoFrame($frame->getData(), $frame->getTimestamp(), $source, $slot, CallRecorder::codecIdOf($frame->getMimeType()));
445445
}
446446
}
447447
}
@@ -483,25 +483,29 @@ public function pushAudioFrame(string $data, int $rtpTimestamp, int $source = 0)
483483
* @param int $rtpTimestamp The frame's RTP timestamp (90 kHz clock).
484484
* @param int $source Identifies the track the frame belongs to.
485485
* @param int $slot Whether this is the camera ({@see CallStream::VIDEO}) or the screen share ({@see CallStream::SCREEN}).
486+
* @param ?string $codecId The Matroska codec ID of the frame, if known (see {@see CallRecorder::pushVideoFrame()}).
486487
*/
487-
public function pushVideoFrame(string $data, int $rtpTimestamp, int $source = 0, int $slot = CallStream::VIDEO): void
488+
public function pushVideoFrame(string $data, int $rtpTimestamp, int $source = 0, int $slot = CallStream::VIDEO, ?string $codecId = null): void
488489
{
489490
if ($this->closed || $data === '' || $this->expected[$slot] === false) {
490491
return;
491492
}
492-
if (($this->sources[$slot] ?? null) !== $source) {
493-
// A new source: its codec is read off its first keyframe, like the recorder does.
494-
[, $keyframe] = CallRecorder::transformVideo($data, null);
493+
if (($this->sources[$slot] ?? null) !== $source
494+
|| ($codecId !== null && ($this->codecs[$slot] ?? null) !== $codecId)
495+
) {
496+
// A new source, or a new codec on the same one: the codec is read off its first keyframe,
497+
// like the recorder does.
498+
[, $keyframe] = CallRecorder::transformVideo($data, $codecId);
495499
if ($keyframe) {
496-
[$codecId] = CallRecorder::describeVideo($data);
500+
$codec = $codecId ?? CallRecorder::describeVideo($data)[0];
497501
$this->sources[$slot] = $source;
498-
if (($this->codecs[$slot] ?? null) !== $codecId) {
499-
$this->codecs[$slot] = $codecId;
502+
if (($this->codecs[$slot] ?? null) !== $codec) {
503+
$this->codecs[$slot] = $codec;
500504
$this->report(null, null);
501505
}
502506
}
503507
}
504-
$this->recorder?->pushVideoFrame($data, $rtpTimestamp, $source, $slot);
508+
$this->recorder?->pushVideoFrame($data, $rtpTimestamp, $source, $slot, $codecId);
505509
}
506510

507511
/**

‎tests/danog/MadelineProto/CallRecorderSegmentsTest.php‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,54 @@ public function testCodecChangeStartsANewSegment(): void
181181
$this->assertSame(1, self::describe($this->segment(1, 'video'))['frames']);
182182
}
183183

184+
/** A 32x32 H.264 keyframe (SPS, PPS and IDR slice, with start codes) and the inter frame after it. */
185+
private const H264_KEYFRAME = '000000016742c00ad9096c0440000003004000000f03c489920000000168cb83cb200000016588840af118a00024031c00043ae3800097c49d75e0';
186+
private const H264_INTER = '00000001419a3813e580';
187+
188+
/**
189+
* @return list<array{codec: string, keyframe: bool}>
190+
*/
191+
private static function frames(string $path): array
192+
{
193+
$frames = [];
194+
foreach ((new Matroska(new LocalFile($path), null))->frames as $frame) {
195+
$frames[] = ['codec' => $frame['codec'], 'keyframe' => $frame['keyframe']];
196+
}
197+
return $frames;
198+
}
199+
200+
/**
201+
* A sender can switch codec on the same SSRC (as MadelineProto itself does in group calls, when the next file
202+
* has another video codec): the codec of the payload type of the frames is what tells, as an inter frame of one
203+
* codec can look like another's. The frames of the new codec go into a new segment, parsed as what they are.
204+
*/
205+
public function testCodecChangeOnTheSameSourceStartsANewSegment(): void
206+
{
207+
$recorder = CallRecorder::series(new LocalFile($this->base), RecordingFormat::Mkv);
208+
$recorder->setExpected(audio: false, video: true, presentation: false);
209+
$recorder->pushVideoFrame(self::vp9(true), 0, 1, codecId: 'V_VP9');
210+
$recorder->pushVideoFrame(self::vp9(false), 3000, 1, codecId: 'V_VP9');
211+
$recorder->pushVideoFrame(hex2bin(self::H264_KEYFRAME), 6000, 1, codecId: 'V_MPEG4/ISO/AVC');
212+
$this->assertFileExists($this->segment(1, 'video'));
213+
$recorder->pushVideoFrame(hex2bin(self::H264_INTER), 9000, 1, codecId: 'V_MPEG4/ISO/AVC');
214+
$recorder->close();
215+
$this->assertSame([['codec' => 'V_VP9', 'keyframe' => true], ['codec' => 'V_VP9', 'keyframe' => false]], self::frames($this->segment(0, 'video')));
216+
$this->assertSame([['codec' => 'V_MPEG4/ISO/AVC', 'keyframe' => true], ['codec' => 'V_MPEG4/ISO/AVC', 'keyframe' => false]], self::frames($this->segment(1, 'video')));
217+
}
218+
219+
/** A single-file recording can't change codec either: it's finished, rather than given frames of another codec. */
220+
public function testCodecChangeOnTheSameSourceFinishesAFixedRecording(): void
221+
{
222+
$recorder = CallRecorder::fixed(new LocalFile($this->base), RecordingFormat::Mkv, CallStream::VIDEO);
223+
$recorder->setExpected(audio: false, video: true, presentation: false);
224+
$recorder->pushVideoFrame(self::vp9(true), 0, 1, codecId: 'V_VP9');
225+
$recorder->pushVideoFrame(self::vp9(false), 3000, 1, codecId: 'V_VP9');
226+
$recorder->pushVideoFrame(hex2bin(self::H264_KEYFRAME), 6000, 1, codecId: 'V_MPEG4/ISO/AVC');
227+
$recorder->pushVideoFrame(hex2bin(self::H264_INTER), 9000, 1, codecId: 'V_MPEG4/ISO/AVC');
228+
$recorder->close();
229+
$this->assertSame([['codec' => 'V_VP9', 'keyframe' => true], ['codec' => 'V_VP9', 'keyframe' => false]], self::frames($this->base));
230+
}
231+
184232
public function testDirectoryStemAndNothingRecorded(): void
185233
{
186234
$dir = sys_get_temp_dir().'/rec_'.bin2hex(random_bytes(4)).'/';

‎tests/danog/MadelineProto/IncomingMediaTest.php‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,22 @@ public function testStreamsAndCodecsAreReportedOnce(): void
9898
], $this->events);
9999
}
100100

101+
/** A codec switch on the same source (told by the payload type of the frames) is reported once its keyframe arrives. */
102+
public function testACodecChangeOnTheSameSourceIsReported(): void
103+
{
104+
$this->media->setExpected(audio: false, video: true, presentation: false);
105+
$this->media->pushVideoFrame(self::vp8(true), 0, 10, codecId: 'V_VP8');
106+
$this->media->pushVideoFrame(self::vp8(false), 3000, 10, codecId: 'V_VP8');
107+
$this->media->pushVideoFrame("\x86\x00\x40\x92".str_repeat('v', 20), 6000, 10, codecId: 'V_VP9'); // no keyframe yet
108+
$this->media->pushVideoFrame("\x82\x49\x83\x42\x00\x01\x3f\x00\xb3".str_repeat('v', 30), 9000, 10, codecId: 'V_VP9');
109+
$this->media->pushVideoFrame("\x86\x00\x40\x92".str_repeat('v', 20), 12000, 10, codecId: 'V_VP9');
110+
$this->assertSame([
111+
[CallStream::VIDEO, [], null, null],
112+
[CallStream::VIDEO, [CallStream::VIDEO => 'V_VP8'], null, null],
113+
[CallStream::VIDEO, [CallStream::VIDEO => 'V_VP9'], null, null],
114+
], $this->events);
115+
}
116+
101117
public function testRecordingEventsCarryTheFile(): void
102118
{
103119
$this->media->setExpected(audio: true, video: false, presentation: false);

0 commit comments

Comments
 (0)