From 0239456b21244fc19aef7650f2a1b5b105e632b3 Mon Sep 17 00:00:00 2001 From: Nicolas Clerc Date: Thu, 17 Oct 2024 18:22:27 +0200 Subject: [PATCH 1/8] =?UTF-8?q?=E2=AC=86=EF=B8=8F(back)=20upgrade=20django?= =?UTF-8?q?=5Fpeertube=5Frunner=5Fconnection=20to=20v0.10.0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit https://github.com/openfun/django-peertube-runner-connector/releases/ tag/v0.10.0 --- src/backend/setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/setup.cfg b/src/backend/setup.cfg index a4eba094da..ece62374f8 100644 --- a/src/backend/setup.cfg +++ b/src/backend/setup.cfg @@ -45,7 +45,7 @@ install_requires = django-redis==5.4.0 django-safedelete==1.4.0 django-storages==1.14.3 - django-peertube-runner-connector==0.9.0 + django-peertube-runner-connector==0.10.0 django-waffle==4.1.0 Django<5 djangorestframework==3.15.2 From 28f63f7c1967b2a2ce2069168402cb0e60b31149 Mon Sep 17 00:00:00 2001 From: Nicolas Clerc Date: Fri, 11 Oct 2024 14:04:42 +0200 Subject: [PATCH 2/8] first implementation --- .../marsha/core/tests/utils/test_transcode.py | 33 +++++++++++++++++++ src/backend/marsha/core/utils/transcode.py | 19 +++++++++-- 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/src/backend/marsha/core/tests/utils/test_transcode.py b/src/backend/marsha/core/tests/utils/test_transcode.py index bee368f51b..9ceaa7d182 100644 --- a/src/backend/marsha/core/tests/utils/test_transcode.py +++ b/src/backend/marsha/core/tests/utils/test_transcode.py @@ -7,6 +7,7 @@ VideoFile, VideoState, VideoStreamingPlaylist, + VideoResolution, ) from marsha.core import defaults @@ -31,6 +32,16 @@ def setUp(self): state=VideoState.PUBLISHED, directory=f"vod/{self.video.pk}/video/1698941501", ) + # A temporary source video file should exist on tmp directory + self.transcoded_video_source = VideoFile.objects.create( + video=self.transcoded_video, + streamingPlaylist=None, + resolution=VideoResolution.H_NOVIDEO, + size=123456, + extname="", + filename=f"tmp/{self.video.pk}/video/1698941501", + ) + self.video_playlist = VideoStreamingPlaylist.objects.create( video=self.transcoded_video ) @@ -63,6 +74,17 @@ def test_transcoding_ended_callback(self): self.assertEqual(self.video.upload_state, defaults.READY) self.assertEqual(self.video.transcode_pipeline, defaults.PEERTUBE_PIPELINE) + # Temporary source video file should be deleted + with self.assertRaises(VideoFile.DoesNotExist): + VideoFile.objects.get(id=self.transcoded_video_source.id) + + self.assertEqual(self.transcoded_video.files.count(), 2) + self.assertQuerySetEqual( + self.transcoded_video.files.all(), + [self.video_file1, self.video_file2], + ordered=False, + ) + def test_transcoding_ended_callback_with_error(self): """The marsha video should be set with state on error and nothing else should be done.""" self.transcoded_video.state = VideoState.TRANSCODING_FAILED @@ -78,3 +100,14 @@ def test_transcoding_ended_callback_with_error(self): self.assertEqual(self.video.resolutions, []) self.assertEqual(self.video.transcode_pipeline, None) + + # Temporary source video file should be deleted + with self.assertRaises(VideoFile.DoesNotExist): + VideoFile.objects.get(id=self.transcoded_video_source.id) + + self.assertEqual(self.transcoded_video.files.count(), 2) + self.assertQuerySetEqual( + self.transcoded_video.files.all(), + [self.video_file1, self.video_file2], + ordered=False, + ) diff --git a/src/backend/marsha/core/utils/transcode.py b/src/backend/marsha/core/utils/transcode.py index dc1e916c23..fc64bbb4a0 100644 --- a/src/backend/marsha/core/utils/transcode.py +++ b/src/backend/marsha/core/utils/transcode.py @@ -1,6 +1,11 @@ """ Utils related to transcoding """ -from django_peertube_runner_connector.models import Video as TranscodedVideo, VideoState +from django_peertube_runner_connector.models import ( + Video as TranscodedVideo, + VideoState, + VideoFile, + VideoResolution, +) from marsha.core.defaults import ERROR, PEERTUBE_PIPELINE, READY from marsha.core.models.video import Video @@ -9,7 +14,7 @@ def transcoding_ended_callback(transcoded_video: TranscodedVideo): """ - Callback used when the a Peertube runnner has finished + Callback used when a Peertube runner has finished to transcode a video. Parameters @@ -24,6 +29,16 @@ def transcoding_ended_callback(transcoded_video: TranscodedVideo): video_id = directory[-3] video = Video.objects.get(pk=video_id) + temp_video_file = VideoFile.objects.get( + video=transcoded_video, + streamingPlaylist=None, + resolution=VideoResolution.H_NOVIDEO, + extname="", + filename=f"tmp/{video_id}/video/{uploaded_on}", + ) + temp_video_file.remove_web_video_file() + temp_video_file.delete() + if transcoded_video.state == VideoState.TRANSCODING_FAILED: video.update_upload_state(ERROR, None) return From 34ccd1e61634ed1aecac3f34fc5dd165d9c38172 Mon Sep 17 00:00:00 2001 From: Nicolas Clerc Date: Mon, 14 Oct 2024 10:19:03 +0200 Subject: [PATCH 3/8] fixup! first implementation --- .../marsha/core/tests/utils/test_transcode.py | 49 ++++--------------- src/backend/marsha/core/utils/transcode.py | 29 +++++------ 2 files changed, 23 insertions(+), 55 deletions(-) diff --git a/src/backend/marsha/core/tests/utils/test_transcode.py b/src/backend/marsha/core/tests/utils/test_transcode.py index 9ceaa7d182..189a1e4ae2 100644 --- a/src/backend/marsha/core/tests/utils/test_transcode.py +++ b/src/backend/marsha/core/tests/utils/test_transcode.py @@ -1,5 +1,7 @@ """Tests for the `core.utils.transcode` module.""" +from unittest.mock import patch + from django.test import TestCase from django_peertube_runner_connector.models import ( @@ -7,7 +9,6 @@ VideoFile, VideoState, VideoStreamingPlaylist, - VideoResolution, ) from marsha.core import defaults @@ -32,15 +33,6 @@ def setUp(self): state=VideoState.PUBLISHED, directory=f"vod/{self.video.pk}/video/1698941501", ) - # A temporary source video file should exist on tmp directory - self.transcoded_video_source = VideoFile.objects.create( - video=self.transcoded_video, - streamingPlaylist=None, - resolution=VideoResolution.H_NOVIDEO, - size=123456, - extname="", - filename=f"tmp/{self.video.pk}/video/1698941501", - ) self.video_playlist = VideoStreamingPlaylist.objects.create( video=self.transcoded_video @@ -61,53 +53,32 @@ def setUp(self): extname="mp4", ) - def test_transcoding_ended_callback(self): + @patch("marsha.core.utils.transcode.delete_temp_file") + def test_transcoding_ended_callback(self, mock_delete_temp_file): """The marsha video should correctly be updated.""" transcoding_ended_callback(self.transcoded_video) self.video.refresh_from_db() - self.assertEqual(self.video.uploaded_on, to_datetime("1698941501")) - self.assertEqual(self.video.resolutions, [720, 1080]) - self.assertEqual(self.video.upload_state, defaults.READY) self.assertEqual(self.video.transcode_pipeline, defaults.PEERTUBE_PIPELINE) - - # Temporary source video file should be deleted - with self.assertRaises(VideoFile.DoesNotExist): - VideoFile.objects.get(id=self.transcoded_video_source.id) - - self.assertEqual(self.transcoded_video.files.count(), 2) - self.assertQuerySetEqual( - self.transcoded_video.files.all(), - [self.video_file1, self.video_file2], - ordered=False, + mock_delete_temp_file.assert_called_once_with( + self.transcoded_video, f"tmp/{self.video.pk}/video/1698941501" ) - def test_transcoding_ended_callback_with_error(self): + @patch("marsha.core.utils.transcode.delete_temp_file") + def test_transcoding_ended_callback_with_error(self, mock_delete_temp_file): """The marsha video should be set with state on error and nothing else should be done.""" self.transcoded_video.state = VideoState.TRANSCODING_FAILED transcoding_ended_callback(self.transcoded_video) self.video.refresh_from_db() - self.assertEqual(self.video.upload_state, defaults.ERROR) - self.assertEqual(self.video.uploaded_on, None) - self.assertEqual(self.video.resolutions, []) - self.assertEqual(self.video.transcode_pipeline, None) - - # Temporary source video file should be deleted - with self.assertRaises(VideoFile.DoesNotExist): - VideoFile.objects.get(id=self.transcoded_video_source.id) - - self.assertEqual(self.transcoded_video.files.count(), 2) - self.assertQuerySetEqual( - self.transcoded_video.files.all(), - [self.video_file1, self.video_file2], - ordered=False, + mock_delete_temp_file.assert_called_once_with( + self.transcoded_video, f"tmp/{self.video.pk}/video/1698941501" ) diff --git a/src/backend/marsha/core/utils/transcode.py b/src/backend/marsha/core/utils/transcode.py index fc64bbb4a0..cdd1c7715f 100644 --- a/src/backend/marsha/core/utils/transcode.py +++ b/src/backend/marsha/core/utils/transcode.py @@ -1,13 +1,15 @@ """ Utils related to transcoding """ -from django_peertube_runner_connector.models import ( - Video as TranscodedVideo, - VideoState, - VideoFile, - VideoResolution, +from django_peertube_runner_connector.models import Video as TranscodedVideo, VideoState +from django_peertube_runner_connector.utils.files import delete_temp_file + +from marsha.core.defaults import ( + ERROR, + PEERTUBE_PIPELINE, + READY, + TMP_VIDEOS_STORAGE_BASE_DIRECTORY, + VOD_VIDEOS_STORAGE_BASE_DIRECTORY, ) - -from marsha.core.defaults import ERROR, PEERTUBE_PIPELINE, READY from marsha.core.models.video import Video from marsha.core.utils.time_utils import to_datetime @@ -28,16 +30,11 @@ def transcoding_ended_callback(transcoded_video: TranscodedVideo): uploaded_on = directory[-1] video_id = directory[-3] video = Video.objects.get(pk=video_id) - - temp_video_file = VideoFile.objects.get( - video=transcoded_video, - streamingPlaylist=None, - resolution=VideoResolution.H_NOVIDEO, - extname="", - filename=f"tmp/{video_id}/video/{uploaded_on}", + tmp_filename = transcoded_video.directory.replace( + VOD_VIDEOS_STORAGE_BASE_DIRECTORY, TMP_VIDEOS_STORAGE_BASE_DIRECTORY ) - temp_video_file.remove_web_video_file() - temp_video_file.delete() + + delete_temp_file(transcoded_video, tmp_filename) if transcoded_video.state == VideoState.TRANSCODING_FAILED: video.update_upload_state(ERROR, None) From 2348be6ceb07495458af9733f0986a6744aec3c1 Mon Sep 17 00:00:00 2001 From: Nicolas Clerc Date: Thu, 17 Oct 2024 17:05:04 +0200 Subject: [PATCH 4/8] fixup! first implementation --- .../commands/delete_transcoding_temp_files.py | 20 ++++++ .../test_delete_transcoding_temp_files.py | 23 +++++++ .../marsha/core/tests/utils/test_transcode.py | 62 ++++++++++++++++++- src/backend/marsha/core/utils/transcode.py | 12 ++++ 4 files changed, 116 insertions(+), 1 deletion(-) create mode 100644 src/backend/marsha/core/management/commands/delete_transcoding_temp_files.py create mode 100644 src/backend/marsha/core/tests/management_commands/test_delete_transcoding_temp_files.py diff --git a/src/backend/marsha/core/management/commands/delete_transcoding_temp_files.py b/src/backend/marsha/core/management/commands/delete_transcoding_temp_files.py new file mode 100644 index 0000000000..02a986ff03 --- /dev/null +++ b/src/backend/marsha/core/management/commands/delete_transcoding_temp_files.py @@ -0,0 +1,20 @@ +"""Management command to delete transcoding temp files.""" + +import logging + +from django.core.management import BaseCommand + +from marsha.core.utils.transcode import delete_transcoding_temp_files + + +logger = logging.getLogger(__name__) + + +class Command(BaseCommand): + """Delete transcoding temp files.""" + + help = "Delete transcoding temp files" + + def handle(self, *args, **options): + """Delete all transcoding temp files.""" + delete_transcoding_temp_files() diff --git a/src/backend/marsha/core/tests/management_commands/test_delete_transcoding_temp_files.py b/src/backend/marsha/core/tests/management_commands/test_delete_transcoding_temp_files.py new file mode 100644 index 0000000000..f90b7faf96 --- /dev/null +++ b/src/backend/marsha/core/tests/management_commands/test_delete_transcoding_temp_files.py @@ -0,0 +1,23 @@ +"""Test transcript_video command.""" + +from unittest.mock import patch + +from django.core.management import call_command +from django.test import TestCase + +from marsha.core.utils import transcode + + +@patch.object(transcode, "delete_transcoding_temp_files") +class TranscriptVideosTestCase(TestCase): + """ + Test case for the transcript_videos command. + """ + + maxDiff = None + + def test_delete_transcoding_temp_files(self, mock_delete_temp_files): + """Should call delete_transcoding_temp_files function.""" + call_command("delete_transcoding_temp_files") + + mock_delete_temp_files.assert_called_once() diff --git a/src/backend/marsha/core/tests/utils/test_transcode.py b/src/backend/marsha/core/tests/utils/test_transcode.py index 189a1e4ae2..45e9e3fd17 100644 --- a/src/backend/marsha/core/tests/utils/test_transcode.py +++ b/src/backend/marsha/core/tests/utils/test_transcode.py @@ -4,6 +4,10 @@ from django.test import TestCase +from django_peertube_runner_connector.factories import ( + VideoFactory as TranscodedVideoFactory, + VideoJobInfoFactory, +) from django_peertube_runner_connector.models import ( Video as TranscodedVideo, VideoFile, @@ -14,7 +18,10 @@ from marsha.core import defaults from marsha.core.factories import VideoFactory from marsha.core.utils.time_utils import to_datetime -from marsha.core.utils.transcode import transcoding_ended_callback +from marsha.core.utils.transcode import ( + delete_transcoding_temp_files, + transcoding_ended_callback, +) class TranscodeTestCase(TestCase): @@ -82,3 +89,56 @@ def test_transcoding_ended_callback_with_error(self, mock_delete_temp_file): mock_delete_temp_file.assert_called_once_with( self.transcoded_video, f"tmp/{self.video.pk}/video/1698941501" ) + + @patch("marsha.core.utils.transcode.delete_temp_file") + def test_transcoding_delete_transcoding_temp_files(self, mock_delete_temp_file): + """The temp files should be deleted.""" + VideoJobInfoFactory(pendingTranscode=0, video=self.transcoded_video) + temp_video_file = TranscodedVideoFactory( + directory=self.transcoded_video.directory.replace( + defaults.VOD_VIDEOS_STORAGE_BASE_DIRECTORY, + defaults.TMP_VIDEOS_STORAGE_BASE_DIRECTORY, + ) + ) + + delete_transcoding_temp_files() + + mock_delete_temp_file.assert_called_once_with( + self.transcoded_video, temp_video_file.directory + ) + + @patch("marsha.core.utils.transcode.delete_temp_file") + def test_transcoding_delete_transcoding_temp_files_pending_transcode( + self, mock_delete_temp_file + ): + """The temp files should not be deleted if there is a pending transcode.""" + VideoJobInfoFactory(pendingTranscode=1, video=self.transcoded_video) + TranscodedVideoFactory( + directory=self.transcoded_video.directory.replace( + defaults.VOD_VIDEOS_STORAGE_BASE_DIRECTORY, + defaults.TMP_VIDEOS_STORAGE_BASE_DIRECTORY, + ) + ) + + delete_transcoding_temp_files() + + mock_delete_temp_file.assert_not_called() + + @patch("marsha.core.utils.transcode.delete_temp_file") + def test_transcoding_delete_transcoding_temp_files_not_published( + self, mock_delete_temp_file + ): + """The temp files should be deleted.""" + self.transcoded_video.state = VideoState.TO_TRANSCODE + self.transcoded_video.save() + VideoJobInfoFactory(pendingTranscode=0, video=self.transcoded_video) + TranscodedVideoFactory( + directory=self.transcoded_video.directory.replace( + defaults.VOD_VIDEOS_STORAGE_BASE_DIRECTORY, + defaults.TMP_VIDEOS_STORAGE_BASE_DIRECTORY, + ) + ) + + delete_transcoding_temp_files() + + mock_delete_temp_file.assert_not_called() diff --git a/src/backend/marsha/core/utils/transcode.py b/src/backend/marsha/core/utils/transcode.py index cdd1c7715f..5e3e7872d7 100644 --- a/src/backend/marsha/core/utils/transcode.py +++ b/src/backend/marsha/core/utils/transcode.py @@ -51,3 +51,15 @@ def transcoding_ended_callback(transcoded_video: TranscodedVideo): to_datetime(uploaded_on), **{"resolutions": resolutions}, ) + + +def delete_transcoding_temp_files(): + """Delete all transcoding temp files.""" + transcoded_videos = TranscodedVideo.objects.filter( + state=VideoState.PUBLISHED, jobInfo__pendingTranscode=0 + ) + for transcoded_video in transcoded_videos: + tmp_filename = transcoded_video.directory.replace( + VOD_VIDEOS_STORAGE_BASE_DIRECTORY, TMP_VIDEOS_STORAGE_BASE_DIRECTORY + ) + delete_temp_file(transcoded_video, tmp_filename) From e9b8476fb7bd81af20d23b2d9982933eb8d9304b Mon Sep 17 00:00:00 2001 From: Nicolas Clerc Date: Thu, 17 Oct 2024 17:21:00 +0200 Subject: [PATCH 5/8] fixup! first implementation --- .../marsha/core/tests/utils/test_transcode.py | 56 +++++-------------- src/backend/marsha/core/utils/transcode.py | 5 +- 2 files changed, 16 insertions(+), 45 deletions(-) diff --git a/src/backend/marsha/core/tests/utils/test_transcode.py b/src/backend/marsha/core/tests/utils/test_transcode.py index 45e9e3fd17..3bcee1ea8d 100644 --- a/src/backend/marsha/core/tests/utils/test_transcode.py +++ b/src/backend/marsha/core/tests/utils/test_transcode.py @@ -1,12 +1,12 @@ """Tests for the `core.utils.transcode` module.""" from unittest.mock import patch +from uuid import uuid4 from django.test import TestCase from django_peertube_runner_connector.factories import ( VideoFactory as TranscodedVideoFactory, - VideoJobInfoFactory, ) from django_peertube_runner_connector.models import ( Video as TranscodedVideo, @@ -24,8 +24,8 @@ ) -class TranscodeTestCase(TestCase): - """Test the `transcode` functions.""" +class TranscodingEndedCallbackTestCase(TestCase): + """Test the `transcoding_ended_callback` function.""" def setUp(self): # Create a test video @@ -90,55 +90,29 @@ def test_transcoding_ended_callback_with_error(self, mock_delete_temp_file): self.transcoded_video, f"tmp/{self.video.pk}/video/1698941501" ) + +class DeleteTranscodingTempFilesTestCase(TestCase): + """Test the `delete_transcoding_temp_files` function.""" + @patch("marsha.core.utils.transcode.delete_temp_file") def test_transcoding_delete_transcoding_temp_files(self, mock_delete_temp_file): """The temp files should be deleted.""" - VideoJobInfoFactory(pendingTranscode=0, video=self.transcoded_video) - temp_video_file = TranscodedVideoFactory( - directory=self.transcoded_video.directory.replace( - defaults.VOD_VIDEOS_STORAGE_BASE_DIRECTORY, - defaults.TMP_VIDEOS_STORAGE_BASE_DIRECTORY, - ) + video_id = uuid4() + video_file = TranscodedVideoFactory( + directory=f"vod/{video_id}/video/1698941501" ) delete_transcoding_temp_files() mock_delete_temp_file.assert_called_once_with( - self.transcoded_video, temp_video_file.directory - ) - - @patch("marsha.core.utils.transcode.delete_temp_file") - def test_transcoding_delete_transcoding_temp_files_pending_transcode( - self, mock_delete_temp_file - ): - """The temp files should not be deleted if there is a pending transcode.""" - VideoJobInfoFactory(pendingTranscode=1, video=self.transcoded_video) - TranscodedVideoFactory( - directory=self.transcoded_video.directory.replace( - defaults.VOD_VIDEOS_STORAGE_BASE_DIRECTORY, - defaults.TMP_VIDEOS_STORAGE_BASE_DIRECTORY, - ) + video_file, f"tmp/{video_id}/video/1698941501" ) - delete_transcoding_temp_files() - - mock_delete_temp_file.assert_not_called() - @patch("marsha.core.utils.transcode.delete_temp_file") - def test_transcoding_delete_transcoding_temp_files_not_published( - self, mock_delete_temp_file - ): - """The temp files should be deleted.""" - self.transcoded_video.state = VideoState.TO_TRANSCODE - self.transcoded_video.save() - VideoJobInfoFactory(pendingTranscode=0, video=self.transcoded_video) - TranscodedVideoFactory( - directory=self.transcoded_video.directory.replace( - defaults.VOD_VIDEOS_STORAGE_BASE_DIRECTORY, - defaults.TMP_VIDEOS_STORAGE_BASE_DIRECTORY, - ) - ) + def test_transcoding_delete_transcoding_temp_files_all(self, mock_delete_temp_file): + """All video files should be processed.""" + TranscodedVideoFactory.create_batch(15) delete_transcoding_temp_files() - mock_delete_temp_file.assert_not_called() + self.assertEqual(mock_delete_temp_file.call_count, 15) diff --git a/src/backend/marsha/core/utils/transcode.py b/src/backend/marsha/core/utils/transcode.py index 5e3e7872d7..530d127bbf 100644 --- a/src/backend/marsha/core/utils/transcode.py +++ b/src/backend/marsha/core/utils/transcode.py @@ -55,10 +55,7 @@ def transcoding_ended_callback(transcoded_video: TranscodedVideo): def delete_transcoding_temp_files(): """Delete all transcoding temp files.""" - transcoded_videos = TranscodedVideo.objects.filter( - state=VideoState.PUBLISHED, jobInfo__pendingTranscode=0 - ) - for transcoded_video in transcoded_videos: + for transcoded_video in TranscodedVideo.objects.all(): tmp_filename = transcoded_video.directory.replace( VOD_VIDEOS_STORAGE_BASE_DIRECTORY, TMP_VIDEOS_STORAGE_BASE_DIRECTORY ) From 8d133029aed5d6441446f56de7270e5ba630cd26 Mon Sep 17 00:00:00 2001 From: Nicolas Clerc Date: Mon, 21 Oct 2024 13:06:25 +0200 Subject: [PATCH 6/8] generate transcripts as subtitles (management command) --- .../core/tests/utils/test_transcript.py | 23 +++++++++++-------- .../marsha/core/utils/transcript_utils.py | 10 ++++---- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/backend/marsha/core/tests/utils/test_transcript.py b/src/backend/marsha/core/tests/utils/test_transcript.py index 653e305b3d..9453daf68d 100644 --- a/src/backend/marsha/core/tests/utils/test_transcript.py +++ b/src/backend/marsha/core/tests/utils/test_transcript.py @@ -56,10 +56,13 @@ def test_transcription_ended_callback(self): timed_text_track = video.timedtexttracks.get() self.assertEqual(timed_text_track.language, language) - self.assertEqual(timed_text_track.mode, TimedTextTrack.TRANSCRIPT) + self.assertEqual(timed_text_track.mode, TimedTextTrack.SUBTITLE) self.assertEqual(timed_text_track.upload_state, defaults.READY) self.assertEqual(timed_text_track.extension, "vtt") + video.refresh_from_db() + self.assertTrue(video.should_use_subtitle_as_transcript) + ttt_path = timed_text_track.get_videos_storage_prefix() self.assertTrue( video_storage.exists(f"{ttt_path}/source.{timed_text_track.extension}") @@ -114,15 +117,15 @@ def test_transcript_video_no_video(self, mock_launch_video_transcript): mock_launch_video_transcript.delay.assert_not_called() @patch.object(transcript_utils, "launch_video_transcript") - def test_transcript_video_already_transcript(self, mock_launch_video_transcript): + def test_transcript_video_already_subtitle(self, mock_launch_video_transcript): """ Should not call the launch_video_transcript function - if the video already has a transcript. + if the video already has a subtitle. """ timed_text_track = TimedTextTrackFactory( video=VideoFactory(upload_state=defaults.READY), language=settings.LANGUAGES[0][0], - mode=TimedTextTrack.TRANSCRIPT, + mode=TimedTextTrack.SUBTITLE, ) with self.assertRaises(transcript_utils.TranscriptError) as context: @@ -130,15 +133,15 @@ def test_transcript_video_already_transcript(self, mock_launch_video_transcript) self.assertEqual( str(context.exception), - f"A transcript already exists for video {timed_text_track.video.id}", + f"A subtitle already exists for video {timed_text_track.video.id}", ) mock_launch_video_transcript.delay.assert_not_called() @patch.object(transcript_utils, "launch_video_transcript") - def test_transcript_video_already_subtitle(self, mock_launch_video_transcript): + def test_transcript_video_already_transcript(self, mock_launch_video_transcript): """ Should call the launch_video_transcript function - if the video has a subtitle. + if the video has a transcript. """ timed_text_track = TimedTextTrackFactory( video=VideoFactory( @@ -146,7 +149,7 @@ def test_transcript_video_already_subtitle(self, mock_launch_video_transcript): transcode_pipeline=defaults.PEERTUBE_PIPELINE, ), language=settings.LANGUAGES[0][0], - mode=TimedTextTrack.SUBTITLE, + mode=TimedTextTrack.TRANSCRIPT, ) transcript_utils.transcript(timed_text_track.video) @@ -159,7 +162,7 @@ def test_transcript_video_already_subtitle(self, mock_launch_video_transcript): self.assertEqual(timed_text_track.video.timedtexttracks.count(), 2) self.assertTrue( timed_text_track.video.timedtexttracks.filter( - mode=TimedTextTrack.TRANSCRIPT + mode=TimedTextTrack.SUBTITLE ).exists() ) @@ -190,7 +193,7 @@ def test_transcript_video_already_closed_caption( self.assertEqual(timed_text_track.video.timedtexttracks.count(), 2) self.assertTrue( timed_text_track.video.timedtexttracks.filter( - mode=TimedTextTrack.TRANSCRIPT + mode=TimedTextTrack.SUBTITLE ).exists() ) diff --git a/src/backend/marsha/core/utils/transcript_utils.py b/src/backend/marsha/core/utils/transcript_utils.py index 2312d07267..f33809d74e 100644 --- a/src/backend/marsha/core/utils/transcript_utils.py +++ b/src/backend/marsha/core/utils/transcript_utils.py @@ -28,13 +28,11 @@ def transcript(video): TimedTextTrack.objects.create( video=video, language=settings.LANGUAGES[0][0], - mode=TimedTextTrack.TRANSCRIPT, + mode=TimedTextTrack.SUBTITLE, upload_state=defaults.PROCESSING, ) except IntegrityError as e: - raise TranscriptError( - f"A transcript already exists for video {video.id}" - ) from e + raise TranscriptError(f"A subtitle already exists for video {video.id}") from e domain = ( settings.TRANSCODING_CALLBACK_DOMAIN @@ -78,7 +76,7 @@ def transcription_ended_callback( timed_text_track, created = video.timedtexttracks.get_or_create( upload_state=defaults.PROCESSING, - mode=TimedTextTrack.TRANSCRIPT, + mode=TimedTextTrack.SUBTITLE, defaults={ "language": language, "extension": "vtt", @@ -86,6 +84,8 @@ def transcription_ended_callback( "upload_state": defaults.READY, }, ) + video.should_use_subtitle_as_transcript = True + video.save() if not created: timed_text_track.upload_state = defaults.READY timed_text_track.uploaded_on = to_datetime(uploaded_on) From 8dac61a1ca7d25837a03295049af87d1c3a309fa Mon Sep 17 00:00:00 2001 From: Nicolas Clerc Date: Mon, 21 Oct 2024 13:08:46 +0200 Subject: [PATCH 7/8] (backend) generate transcripts as subtitles (button) --- src/backend/marsha/core/api/video.py | 2 +- .../marsha/core/tests/api/video/test_initiate_transcript.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/backend/marsha/core/api/video.py b/src/backend/marsha/core/api/video.py index 510088f3cf..08c5f0b34e 100644 --- a/src/backend/marsha/core/api/video.py +++ b/src/backend/marsha/core/api/video.py @@ -1226,7 +1226,7 @@ def initiate_transcript(self, request, pk=None): timed_text_track = TimedTextTrack.objects.create( video=video, language=settings.LANGUAGES[0][0], - mode=TimedTextTrack.TRANSCRIPT, + mode=TimedTextTrack.SUBTITLE, upload_state=defaults.PROCESSING, ) except IntegrityError: diff --git a/src/backend/marsha/core/tests/api/video/test_initiate_transcript.py b/src/backend/marsha/core/tests/api/video/test_initiate_transcript.py index 7dc33d8556..8ebecd7fce 100644 --- a/src/backend/marsha/core/tests/api/video/test_initiate_transcript.py +++ b/src/backend/marsha/core/tests/api/video/test_initiate_transcript.py @@ -83,7 +83,7 @@ def test_api_video_initiate_transcript_token_user(self): mock_dispatch_video.assert_called_once_with(video) self.assertEqual(timed_text_track.language, "en") - self.assertEqual(timed_text_track.mode, models.TimedTextTrack.TRANSCRIPT) + self.assertEqual(timed_text_track.mode, models.TimedTextTrack.SUBTITLE) self.assertEqual(timed_text_track.upload_state, defaults.PROCESSING) def test_api_video_initiate_transcript_staff_or_user(self): @@ -136,7 +136,7 @@ def test_api_video_initiate_transcript_existing_timed_text_track(self): factories.TimedTextTrackFactory( video=video, language="en", - mode=models.TimedTextTrack.TRANSCRIPT, + mode=models.TimedTextTrack.SUBTITLE, upload_state=defaults.READY, ) jwt_token = InstructorOrAdminLtiTokenFactory(playlist=video.playlist) @@ -204,5 +204,5 @@ def test_api_video_initiate_transcript_aws_pipeline(self): mock_dispatch_video.assert_called_once_with(video) self.assertEqual(timed_text_track.language, "en") - self.assertEqual(timed_text_track.mode, models.TimedTextTrack.TRANSCRIPT) + self.assertEqual(timed_text_track.mode, models.TimedTextTrack.SUBTITLE) self.assertEqual(timed_text_track.upload_state, defaults.PROCESSING) From 108902d8d0704933b3fe7a55924a39411e1a261a Mon Sep 17 00:00:00 2001 From: Nicolas Clerc Date: Mon, 21 Oct 2024 13:09:16 +0200 Subject: [PATCH 8/8] (frontend) generate transcripts as subtitles (button) --- .../TimedTextTrackItem/index.spec.tsx | 54 +++++++++++++++++-- .../TimedTextTrackItem/index.tsx | 2 +- .../index.spec.tsx | 22 ++++---- .../LocalizedTimedTextTrackUpload/index.tsx | 4 +- 4 files changed, 65 insertions(+), 17 deletions(-) diff --git a/src/frontend/packages/lib_video/src/components/common/VideoWidgetProvider/LocalizedTimedTextTrackUpload/TimedTextTrackItem/index.spec.tsx b/src/frontend/packages/lib_video/src/components/common/VideoWidgetProvider/LocalizedTimedTextTrackUpload/TimedTextTrackItem/index.spec.tsx index 072523e299..5a2d26b59e 100644 --- a/src/frontend/packages/lib_video/src/components/common/VideoWidgetProvider/LocalizedTimedTextTrackUpload/TimedTextTrackItem/index.spec.tsx +++ b/src/frontend/packages/lib_video/src/components/common/VideoWidgetProvider/LocalizedTimedTextTrackUpload/TimedTextTrackItem/index.spec.tsx @@ -113,10 +113,11 @@ describe('', () => { ).not.toBeInTheDocument(); }); - it('renders the component for a processing timed text track', async () => { + it('renders the component for a processing transcript', async () => { const mockedTimedTextTrack = timedTextMockFactory({ language: 'fr-FR', upload_state: uploadState.PROCESSING, + mode: timedTextMode.TRANSCRIPT, }); const mockedUploadingObject: UploadingObject = { @@ -159,11 +160,58 @@ describe('', () => { ).not.toBeInTheDocument(); }); - it('renders the component for a processing transcript', async () => { + it('renders the component for a processing closed caption', async () => { const mockedTimedTextTrack = timedTextMockFactory({ language: 'fr-FR', upload_state: uploadState.PROCESSING, - mode: timedTextMode.TRANSCRIPT, + mode: timedTextMode.CLOSED_CAPTIONING, + }); + + const mockedUploadingObject: UploadingObject = { + file: new File([], 'subtitle.srt'), + objectType: modelName.TIMEDTEXTTRACKS, + objectId: mockedTimedTextTrack.id, + progress: 100, + status: UploadManagerStatus.SUCCESS, + }; + + render( + {}, + uploadManagerState: { + [mockedTimedTextTrack.id]: mockedUploadingObject, + }, + }} + > + + + + , + ); + + await screen.findByText('French'); + screen.getByRole('button', { + name: 'Click on this button to delete the timed text track.', + }); + screen.getByText('Processing'); + expect( + screen.queryByRole('button', { + name: 'Click on this button to retry uploading your failed upload.', + }), + ).not.toBeInTheDocument(); + }); + + it('renders the component for a processing subtitle', async () => { + const mockedTimedTextTrack = timedTextMockFactory({ + language: 'fr-FR', + upload_state: uploadState.PROCESSING, + mode: timedTextMode.SUBTITLE, }); const mockedUploadingObject: UploadingObject = { diff --git a/src/frontend/packages/lib_video/src/components/common/VideoWidgetProvider/LocalizedTimedTextTrackUpload/TimedTextTrackItem/index.tsx b/src/frontend/packages/lib_video/src/components/common/VideoWidgetProvider/LocalizedTimedTextTrackUpload/TimedTextTrackItem/index.tsx index 063e16f509..ae3d89a38a 100644 --- a/src/frontend/packages/lib_video/src/components/common/VideoWidgetProvider/LocalizedTimedTextTrackUpload/TimedTextTrackItem/index.tsx +++ b/src/frontend/packages/lib_video/src/components/common/VideoWidgetProvider/LocalizedTimedTextTrackUpload/TimedTextTrackItem/index.tsx @@ -70,7 +70,7 @@ export const TimedTextTrackItem = ({ pad={{ horizontal: 'small', vertical: 'small' }} > {(timedTextTrack.upload_state !== uploadState.PROCESSING || - timedTextTrack.mode !== timedTextMode.TRANSCRIPT) && ( + timedTextTrack.mode !== timedTextMode.SUBTITLE) && ( ', () => { await screen.findByText('French'); screen.getByText('Uploading'); - screen.getByText('English'); + // screen.getByText('English'); screen.getByText('Processing'); screen.getByText('Spanish'); screen.getByText('Uploading'); @@ -307,10 +307,10 @@ describe('', () => { screen.getAllByRole('button', { name: 'Click on this button to delete the timed text track.', }), - ).toHaveLength(3); + ).toHaveLength(2); }); - it('renders a generate transcription button in transcript mode if none exists', async () => { + it('renders a generate transcription button in subtitle mode if none exists', async () => { const mockedVideo = videoMockFactory({ id: '1234', }); @@ -338,7 +338,7 @@ describe('', () => { render( wrapInVideo( , mockedVideo, ), @@ -351,7 +351,7 @@ describe('', () => { await userEvent.click(generateTranscriptButton); screen.getByText( - "By clicking this button, a transcript will be automatically generated. The transcript's language will be the one detected in the video.", + "By clicking this button, a transcript will be automatically generated and used as a subtitle. The transcript's language will be the one detected in the video.", ); expect(fetchMock.calls()).toHaveLength(2); @@ -360,7 +360,7 @@ describe('', () => { ); }); - it('does not render a generate transcription button in transcript mode if one exists', () => { + it('does not render a generate transcription button in subtitle mode if one exists', () => { const mockedVideo = videoMockFactory({ id: '1234', }); @@ -383,7 +383,7 @@ describe('', () => { const mockedTimedTextTrackCompleted = timedTextMockFactory({ language: 'es-ES', - mode: timedTextMode.TRANSCRIPT, + mode: timedTextMode.SUBTITLE, }); useTimedTextTrack.getState().addResource(mockedTimedTextTrackCompleted); @@ -405,7 +405,7 @@ describe('', () => { ).not.toBeInTheDocument(); }); - it('does not render a generate transcription button in subtitle mode', () => { + it('does not render a generate transcription button in transcript mode', () => { const mockedVideo = videoMockFactory({ id: '1234', }); @@ -429,7 +429,7 @@ describe('', () => { render( wrapInVideo( , mockedVideo, ), @@ -478,7 +478,7 @@ describe('', () => { ).not.toBeInTheDocument(); }); - it('does not render a generate transcription button in transcript mode if flag disabled', () => { + it('does not render a generate transcription button in subtitle mode if flag disabled', () => { const mockedVideo = videoMockFactory({ id: '1234', }); @@ -506,7 +506,7 @@ describe('', () => { render( wrapInVideo( , mockedVideo, ), diff --git a/src/frontend/packages/lib_video/src/components/common/VideoWidgetProvider/LocalizedTimedTextTrackUpload/index.tsx b/src/frontend/packages/lib_video/src/components/common/VideoWidgetProvider/LocalizedTimedTextTrackUpload/index.tsx index 2cf8d6ce17..53ee23f93e 100644 --- a/src/frontend/packages/lib_video/src/components/common/VideoWidgetProvider/LocalizedTimedTextTrackUpload/index.tsx +++ b/src/frontend/packages/lib_video/src/components/common/VideoWidgetProvider/LocalizedTimedTextTrackUpload/index.tsx @@ -65,7 +65,7 @@ const messages = defineMessages({ }, transcriptMessage: { defaultMessage: - "By clicking this button, a transcript will be automatically generated. The transcript's language will be the one detected in the video.", + "By clicking this button, a transcript will be automatically generated and used as a subtitle. The transcript's language will be the one detected in the video.", description: 'Message displayed for explaining the transcript generation.', id: 'components.UploadWidgetGeneric.transcriptMessage', }, @@ -251,7 +251,7 @@ export const LocalizedTimedTextTrackUpload = ({ {isFlagEnabled(flags.TRANSCRIPTION) && video.upload_state === uploadState.READY && - timedTextModeWidget === timedTextMode.TRANSCRIPT && + timedTextModeWidget === timedTextMode.SUBTITLE && filteredTimedTextTracks.length === 0 && (