From 9cf7e8c5aafe23de2a6284a4a74d26089cb04fcb Mon Sep 17 00:00:00 2001 From: Zaggy1024 Date: Wed, 9 Nov 2022 19:47:56 -0600 Subject: [PATCH] LibVideo: Reorganize demuxer file hierarchy and rename Matroska files As new demuxers are added, this will get quite full of files, so it'll be good to have a separate folder for these. To avoid too many chained namespaces, the Containers subdirectory is not also a namespace, but the Matroska folder is for the sake of separating the multiple classes for parsed information entering the Video namespace. --- Meta/Lagom/Fuzzers/FuzzMatroskaReader.cpp | 4 +- Tests/LibVideo/TestVP9Decode.cpp | 6 +-- Userland/Applications/VideoPlayer/main.cpp | 2 - Userland/Libraries/LibVideo/CMakeLists.txt | 4 +- .../LibVideo/{ => Containers}/Demuxer.h | 7 ++- .../Matroska/Document.h} | 2 +- .../Matroska}/MatroskaDemuxer.cpp | 14 +++--- .../Matroska}/MatroskaDemuxer.h | 6 +-- .../Matroska/Reader.cpp} | 43 ++++++++++--------- .../Matroska/Reader.h} | 8 ++-- .../Libraries/LibVideo/PlaybackManager.cpp | 5 +-- Userland/Libraries/LibVideo/PlaybackManager.h | 4 +- Userland/Utilities/matroska.cpp | 9 ++-- 13 files changed, 56 insertions(+), 58 deletions(-) rename Userland/Libraries/LibVideo/{ => Containers}/Demuxer.h (90%) rename Userland/Libraries/LibVideo/{MatroskaDocument.h => Containers/Matroska/Document.h} (99%) rename Userland/Libraries/LibVideo/{ => Containers/Matroska}/MatroskaDemuxer.cpp (90%) rename Userland/Libraries/LibVideo/{ => Containers/Matroska}/MatroskaDemuxer.h (93%) rename Userland/Libraries/LibVideo/{MatroskaReader.cpp => Containers/Matroska/Reader.cpp} (93%) rename Userland/Libraries/LibVideo/{MatroskaReader.h => Containers/Matroska/Reader.h} (97%) diff --git a/Meta/Lagom/Fuzzers/FuzzMatroskaReader.cpp b/Meta/Lagom/Fuzzers/FuzzMatroskaReader.cpp index ac59b7c34ce..dff87bf2cc5 100644 --- a/Meta/Lagom/Fuzzers/FuzzMatroskaReader.cpp +++ b/Meta/Lagom/Fuzzers/FuzzMatroskaReader.cpp @@ -4,12 +4,12 @@ * SPDX-License-Identifier: BSD-2-Clause */ -#include +#include #include extern "C" int LLVMFuzzerTestOneInput(u8 const* data, size_t size) { - auto matroska_document = Video::MatroskaReader::parse_matroska_from_data(data, size); + auto matroska_document = Video::Matroska::Reader::parse_matroska_from_data(data, size); if (!matroska_document) return -1; return 0; diff --git a/Tests/LibVideo/TestVP9Decode.cpp b/Tests/LibVideo/TestVP9Decode.cpp index edcb5558901..c1c008e3270 100644 --- a/Tests/LibVideo/TestVP9Decode.cpp +++ b/Tests/LibVideo/TestVP9Decode.cpp @@ -6,14 +6,14 @@ #include -#include +#include #include static void decode_video(StringView path, size_t expected_frame_count) { - auto matroska_document = Video::MatroskaReader::MatroskaReader::parse_matroska_from_file(path); + auto matroska_document = Video::Matroska::Reader::parse_matroska_from_file(path); VERIFY(matroska_document); - auto video_track_optional = matroska_document->track_for_track_type(Video::TrackEntry::TrackType::Video); + auto video_track_optional = matroska_document->track_for_track_type(Video::Matroska::TrackEntry::TrackType::Video); VERIFY(video_track_optional.has_value()); auto video_track_entry = video_track_optional.value(); diff --git a/Userland/Applications/VideoPlayer/main.cpp b/Userland/Applications/VideoPlayer/main.cpp index 07f8d670975..97130750ae7 100644 --- a/Userland/Applications/VideoPlayer/main.cpp +++ b/Userland/Applications/VideoPlayer/main.cpp @@ -4,8 +4,6 @@ * SPDX-License-Identifier: BSD-2-Clause */ -#include "LibVideo/Color/CodingIndependentCodePoints.h" -#include "LibVideo/MatroskaDemuxer.h" #include #include #include diff --git a/Userland/Libraries/LibVideo/CMakeLists.txt b/Userland/Libraries/LibVideo/CMakeLists.txt index 99aafeda341..eb237587d61 100644 --- a/Userland/Libraries/LibVideo/CMakeLists.txt +++ b/Userland/Libraries/LibVideo/CMakeLists.txt @@ -2,8 +2,8 @@ set(SOURCES Color/ColorConverter.cpp Color/ColorPrimaries.cpp Color/TransferCharacteristics.cpp - MatroskaDemuxer.cpp - MatroskaReader.cpp + Containers/Matroska/MatroskaDemuxer.cpp + Containers/Matroska/Reader.cpp PlaybackManager.cpp VideoFrame.cpp VP9/BitStream.cpp diff --git a/Userland/Libraries/LibVideo/Demuxer.h b/Userland/Libraries/LibVideo/Containers/Demuxer.h similarity index 90% rename from Userland/Libraries/LibVideo/Demuxer.h rename to Userland/Libraries/LibVideo/Containers/Demuxer.h index 477f202f90d..9b1ea757eef 100644 --- a/Userland/Libraries/LibVideo/Demuxer.h +++ b/Userland/Libraries/LibVideo/Containers/Demuxer.h @@ -8,10 +8,9 @@ #include #include - -#include "DecoderError.h" -#include "Sample.h" -#include "Track.h" +#include +#include +#include namespace Video { diff --git a/Userland/Libraries/LibVideo/MatroskaDocument.h b/Userland/Libraries/LibVideo/Containers/Matroska/Document.h similarity index 99% rename from Userland/Libraries/LibVideo/MatroskaDocument.h rename to Userland/Libraries/LibVideo/Containers/Matroska/Document.h index ed416be1286..2dfa53bf342 100644 --- a/Userland/Libraries/LibVideo/MatroskaDocument.h +++ b/Userland/Libraries/LibVideo/Containers/Matroska/Document.h @@ -15,7 +15,7 @@ #include #include -namespace Video { +namespace Video::Matroska { struct EBMLHeader { String doc_type; diff --git a/Userland/Libraries/LibVideo/MatroskaDemuxer.cpp b/Userland/Libraries/LibVideo/Containers/Matroska/MatroskaDemuxer.cpp similarity index 90% rename from Userland/Libraries/LibVideo/MatroskaDemuxer.cpp rename to Userland/Libraries/LibVideo/Containers/Matroska/MatroskaDemuxer.cpp index 892c68dd36f..de753aa75fc 100644 --- a/Userland/Libraries/LibVideo/MatroskaDemuxer.cpp +++ b/Userland/Libraries/LibVideo/Containers/Matroska/MatroskaDemuxer.cpp @@ -6,12 +6,12 @@ #include "MatroskaDemuxer.h" -namespace Video { +namespace Video::Matroska { DecoderErrorOr> MatroskaDemuxer::from_file(StringView filename) { // FIXME: MatroskaReader should return errors. - auto nullable_document = MatroskaReader::parse_matroska_from_file(filename); + auto nullable_document = Reader::parse_matroska_from_file(filename); if (!nullable_document) return DecoderError::format(DecoderErrorCategory::IO, "Failed to open matroska from file '{}'", filename); auto document = nullable_document.release_nonnull(); @@ -21,7 +21,7 @@ DecoderErrorOr> MatroskaDemuxer::from_file(String DecoderErrorOr> MatroskaDemuxer::from_data(ReadonlyBytes data) { // FIXME: MatroskaReader should return errors. - auto nullable_document = MatroskaReader::parse_matroska_from_data(data.data(), data.size()); + auto nullable_document = Reader::parse_matroska_from_data(data.data(), data.size()); if (!nullable_document) return DecoderError::format(DecoderErrorCategory::IO, "Failed to open matroska from data"); auto document = nullable_document.release_nonnull(); @@ -30,17 +30,17 @@ DecoderErrorOr> MatroskaDemuxer::from_data(Readon Vector MatroskaDemuxer::get_tracks_for_type(TrackType type) { - Video::TrackEntry::TrackType matroska_track_type; + TrackEntry::TrackType matroska_track_type; switch (type) { case TrackType::Video: - matroska_track_type = Video::TrackEntry::TrackType::Video; + matroska_track_type = TrackEntry::TrackType::Video; break; case TrackType::Audio: - matroska_track_type = Video::TrackEntry::TrackType::Audio; + matroska_track_type = TrackEntry::TrackType::Audio; break; case TrackType::Subtitles: - matroska_track_type = Video::TrackEntry::TrackType::Subtitle; + matroska_track_type = TrackEntry::TrackType::Subtitle; break; } diff --git a/Userland/Libraries/LibVideo/MatroskaDemuxer.h b/Userland/Libraries/LibVideo/Containers/Matroska/MatroskaDemuxer.h similarity index 93% rename from Userland/Libraries/LibVideo/MatroskaDemuxer.h rename to Userland/Libraries/LibVideo/Containers/Matroska/MatroskaDemuxer.h index 17b73f18de8..59b40ef9f09 100644 --- a/Userland/Libraries/LibVideo/MatroskaDemuxer.h +++ b/Userland/Libraries/LibVideo/Containers/Matroska/MatroskaDemuxer.h @@ -7,11 +7,11 @@ #pragma once #include +#include -#include "Demuxer.h" -#include "MatroskaReader.h" +#include "Reader.h" -namespace Video { +namespace Video::Matroska { class MatroskaDemuxer final : public Demuxer { public: diff --git a/Userland/Libraries/LibVideo/MatroskaReader.cpp b/Userland/Libraries/LibVideo/Containers/Matroska/Reader.cpp similarity index 93% rename from Userland/Libraries/LibVideo/MatroskaReader.cpp rename to Userland/Libraries/LibVideo/Containers/Matroska/Reader.cpp index 24036a4cee5..149590f2755 100644 --- a/Userland/Libraries/LibVideo/MatroskaReader.cpp +++ b/Userland/Libraries/LibVideo/Containers/Matroska/Reader.cpp @@ -4,13 +4,14 @@ * SPDX-License-Identifier: BSD-2-Clause */ -#include "MatroskaReader.h" #include #include #include #include -namespace Video { +#include "Reader.h" + +namespace Video::Matroska { #define CHECK_HAS_VALUE(x) \ if (!(x).has_value()) \ @@ -55,7 +56,7 @@ constexpr u32 BIT_DEPTH_ID = 0x6264; constexpr u32 SIMPLE_BLOCK_ID = 0xA3; constexpr u32 TIMESTAMP_ID = 0xE7; -OwnPtr MatroskaReader::parse_matroska_from_file(StringView path) +OwnPtr Reader::parse_matroska_from_file(StringView path) { auto mapped_file_result = Core::MappedFile::map(path); if (mapped_file_result.is_error()) @@ -65,13 +66,13 @@ OwnPtr MatroskaReader::parse_matroska_from_file(StringView pat return parse_matroska_from_data((u8*)mapped_file->data(), mapped_file->size()); } -OwnPtr MatroskaReader::parse_matroska_from_data(u8 const* data, size_t size) +OwnPtr Reader::parse_matroska_from_data(u8 const* data, size_t size) { - MatroskaReader reader(data, size); + Reader reader(data, size); return reader.parse(); } -OwnPtr MatroskaReader::parse() +OwnPtr Reader::parse() { auto first_element_id = m_streamer.read_variable_size_integer(false); dbgln_if(MATROSKA_TRACE_DEBUG, "First element ID is {:#010x}\n", first_element_id.value()); @@ -97,7 +98,7 @@ OwnPtr MatroskaReader::parse() return matroska_document; } -bool MatroskaReader::parse_master_element([[maybe_unused]] StringView element_name, Function element_consumer) +bool Reader::parse_master_element([[maybe_unused]] StringView element_name, Function element_consumer) { auto element_data_size = m_streamer.read_variable_size_integer(); CHECK_HAS_VALUE(element_data_size); @@ -124,7 +125,7 @@ bool MatroskaReader::parse_master_element([[maybe_unused]] StringView element_na return true; } -Optional MatroskaReader::parse_ebml_header() +Optional Reader::parse_ebml_header() { EBMLHeader header; auto success = parse_master_element("Header"sv, [&](u64 element_id) { @@ -150,7 +151,7 @@ Optional MatroskaReader::parse_ebml_header() return header; } -bool MatroskaReader::parse_segment_elements(MatroskaDocument& matroska_document) +bool Reader::parse_segment_elements(MatroskaDocument& matroska_document) { dbgln_if(MATROSKA_DEBUG, "Parsing segment elements"); auto success = parse_master_element("Segment"sv, [&](u64 element_id) { @@ -177,7 +178,7 @@ bool MatroskaReader::parse_segment_elements(MatroskaDocument& matroska_document) return success; } -OwnPtr MatroskaReader::parse_information() +OwnPtr Reader::parse_information() { auto segment_information = make(); auto success = parse_master_element("Segment Information"sv, [&](u64 element_id) { @@ -213,7 +214,7 @@ OwnPtr MatroskaReader::parse_information() return segment_information; } -bool MatroskaReader::parse_tracks(MatroskaDocument& matroska_document) +bool Reader::parse_tracks(MatroskaDocument& matroska_document) { auto success = parse_master_element("Tracks"sv, [&](u64 element_id) { if (element_id == TRACK_ENTRY_ID) { @@ -234,7 +235,7 @@ bool MatroskaReader::parse_tracks(MatroskaDocument& matroska_document) return success; } -OwnPtr MatroskaReader::parse_track_entry() +OwnPtr Reader::parse_track_entry() { auto track_entry = make(); auto success = parse_master_element("Track"sv, [&](u64 element_id) { @@ -283,7 +284,7 @@ OwnPtr MatroskaReader::parse_track_entry() return track_entry; } -Optional MatroskaReader::parse_video_color_information() +Optional Reader::parse_video_color_information() { TrackEntry::ColorFormat color_format {}; @@ -328,7 +329,7 @@ Optional MatroskaReader::parse_video_color_information( return color_format; } -Optional MatroskaReader::parse_video_track_information() +Optional Reader::parse_video_track_information() { TrackEntry::VideoTrack video_track {}; @@ -359,7 +360,7 @@ Optional MatroskaReader::parse_video_track_information() return video_track; } -Optional MatroskaReader::parse_audio_track_information() +Optional Reader::parse_audio_track_information() { TrackEntry::AudioTrack audio_track {}; @@ -386,7 +387,7 @@ Optional MatroskaReader::parse_audio_track_information() return audio_track; } -OwnPtr MatroskaReader::parse_cluster() +OwnPtr Reader::parse_cluster() { auto cluster = make(); @@ -415,7 +416,7 @@ OwnPtr MatroskaReader::parse_cluster() return cluster; } -OwnPtr MatroskaReader::parse_simple_block() +OwnPtr Reader::parse_simple_block() { auto block = make(); @@ -498,7 +499,7 @@ OwnPtr MatroskaReader::parse_simple_block() return block; } -Optional MatroskaReader::read_string_element() +Optional Reader::read_string_element() { auto string_length = m_streamer.read_variable_size_integer(); if (!string_length.has_value() || m_streamer.remaining() < string_length.value()) @@ -508,7 +509,7 @@ Optional MatroskaReader::read_string_element() return string_value; } -Optional MatroskaReader::read_u64_element() +Optional Reader::read_u64_element() { auto integer_length = m_streamer.read_variable_size_integer(); if (!integer_length.has_value() || m_streamer.remaining() < integer_length.value()) @@ -522,7 +523,7 @@ Optional MatroskaReader::read_u64_element() return result; } -Optional MatroskaReader::read_float_element() +Optional Reader::read_float_element() { auto length = m_streamer.read_variable_size_integer(); if (!length.has_value() || m_streamer.remaining() < length.value()) @@ -546,7 +547,7 @@ Optional MatroskaReader::read_float_element() return read_data.double_value; } -bool MatroskaReader::read_unknown_element() +bool Reader::read_unknown_element() { auto element_length = m_streamer.read_variable_size_integer(); if (!element_length.has_value() || m_streamer.remaining() < element_length.value()) diff --git a/Userland/Libraries/LibVideo/MatroskaReader.h b/Userland/Libraries/LibVideo/Containers/Matroska/Reader.h similarity index 97% rename from Userland/Libraries/LibVideo/MatroskaReader.h rename to Userland/Libraries/LibVideo/Containers/Matroska/Reader.h index 7dd140f26d3..039bd916882 100644 --- a/Userland/Libraries/LibVideo/MatroskaReader.h +++ b/Userland/Libraries/LibVideo/Containers/Matroska/Reader.h @@ -6,18 +6,18 @@ #pragma once -#include "MatroskaDocument.h" +#include "Document.h" #include #include #include #include #include -namespace Video { +namespace Video::Matroska { -class MatroskaReader { +class Reader { public: - MatroskaReader(u8 const* data, size_t size) + Reader(u8 const* data, size_t size) : m_streamer(data, size) { } diff --git a/Userland/Libraries/LibVideo/PlaybackManager.cpp b/Userland/Libraries/LibVideo/PlaybackManager.cpp index ef2a9fffca6..4ab2f17209a 100644 --- a/Userland/Libraries/LibVideo/PlaybackManager.cpp +++ b/Userland/Libraries/LibVideo/PlaybackManager.cpp @@ -6,17 +6,16 @@ #include #include -#include +#include #include -#include "MatroskaDemuxer.h" #include "PlaybackManager.h" namespace Video { DecoderErrorOr> PlaybackManager::from_file(Core::Object& event_handler, StringView filename) { - NonnullOwnPtr demuxer = TRY(MatroskaDemuxer::from_file(filename)); + NonnullOwnPtr demuxer = TRY(Matroska::MatroskaDemuxer::from_file(filename)); auto video_tracks = demuxer->get_tracks_for_type(TrackType::Video); if (video_tracks.is_empty()) return DecoderError::with_description(DecoderErrorCategory::Invalid, "No video track is present"sv); diff --git a/Userland/Libraries/LibVideo/PlaybackManager.h b/Userland/Libraries/LibVideo/PlaybackManager.h index 06be0d4921a..e14f84bf0ec 100644 --- a/Userland/Libraries/LibVideo/PlaybackManager.h +++ b/Userland/Libraries/LibVideo/PlaybackManager.h @@ -17,9 +17,9 @@ #include #include #include +#include +#include -#include "Demuxer.h" -#include "MatroskaDocument.h" #include "VideoDecoder.h" namespace Video { diff --git a/Userland/Utilities/matroska.cpp b/Userland/Utilities/matroska.cpp index ef570e16f2e..b5286b7b8a6 100644 --- a/Userland/Utilities/matroska.cpp +++ b/Userland/Utilities/matroska.cpp @@ -1,15 +1,16 @@ /* * Copyright (c) 2021, Hunter Salyer + * Copyright (c) 2022, Gregory Bertilson * * SPDX-License-Identifier: BSD-2-Clause */ #include -#include +#include ErrorOr serenity_main(Main::Arguments) { - auto document = Video::MatroskaReader::parse_matroska_from_file("/home/anon/Videos/test-webm.webm"sv); + auto document = Video::Matroska::Reader::parse_matroska_from_file("/home/anon/Videos/test-webm.webm"sv); if (!document) { return Error::from_string_literal("Failed to parse :("); } @@ -30,10 +31,10 @@ ErrorOr serenity_main(Main::Arguments) outln("\tTrack has Language \"{}\"", track.language().characters()); outln("\tTrack has CodecID \"{}\"", track.codec_id().characters()); - if (track.track_type() == Video::TrackEntry::TrackType::Video) { + if (track.track_type() == Video::Matroska::TrackEntry::TrackType::Video) { auto const video_track = track.video_track().value(); outln("\t\tVideo is {} pixels wide by {} pixels tall", video_track.pixel_width, video_track.pixel_height); - } else if (track.track_type() == Video::TrackEntry::TrackType::Audio) { + } else if (track.track_type() == Video::Matroska::TrackEntry::TrackType::Audio) { auto const audio_track = track.audio_track().value(); outln("\t\tAudio has {} channels with a bit depth of {}", audio_track.channels, audio_track.bit_depth); }