From ccc84174132f85c34ef510e920039b345fbb0831 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Mangano-Tarumi?= Date: Sat, 8 Dec 2018 11:36:10 -0500 Subject: [PATCH] rename the methods of ogg_reader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit read_page → next_page, because it's more consistent with iterators. read_header_packet → process_header_packet, because it doesn't actually *read* anything. --- src/cli.cc | 4 ++-- src/ogg.cc | 4 ++-- src/opustags.h | 12 ++++++++++-- t/ogg.cc | 22 +++++++++++----------- 4 files changed, 25 insertions(+), 17 deletions(-) diff --git a/src/cli.cc b/src/cli.cc index 2b7e115..992cf6d 100644 --- a/src/cli.cc +++ b/src/cli.cc @@ -187,7 +187,7 @@ static ot::status process(ot::ogg_reader& reader, ot::ogg_writer* writer, const /** \todo Become stream-aware instead of counting the pages of all streams together. */ int absolute_page_no; /*< page number in the physical stream, not logical */ for (absolute_page_no = 0;; ++absolute_page_no) { - ot::status rc = reader.read_page(); + ot::status rc = reader.next_page(); if (rc == ot::st::end_of_stream) break; else if (rc != ot::st::ok) @@ -204,7 +204,7 @@ static ot::status process(ot::ogg_reader& reader, ot::ogg_writer* writer, const } } else if (absolute_page_no == 1) { // Comment header ot::opus_tags tags; - rc = reader.read_header_packet( + rc = reader.process_header_packet( [&tags](ogg_packet& p) { return ot::parse_tags(p, tags); }); if (rc != ot::st::ok) return rc; diff --git a/src/ogg.cc b/src/ogg.cc index 456da4a..d7faa13 100644 --- a/src/ogg.cc +++ b/src/ogg.cc @@ -23,7 +23,7 @@ bool ot::is_opus_stream(const ogg_page& identification_header) return (memcmp(identification_header.body, "OpusHead", 8) == 0); } -ot::status ot::ogg_reader::read_page() +ot::status ot::ogg_reader::next_page() { while (ogg_sync_pageout(&sync, &page) != 1) { if (feof(file)) @@ -42,7 +42,7 @@ ot::status ot::ogg_reader::read_page() return st::ok; } -ot::status ot::ogg_reader::read_header_packet(const std::function& f) +ot::status ot::ogg_reader::process_header_packet(const std::function& f) { if (ogg_page_continued(&page)) return {ot::st::error, "Unexpected continued header page."}; diff --git a/src/opustags.h b/src/opustags.h index eefe2f8..d851eb9 100644 --- a/src/opustags.h +++ b/src/opustags.h @@ -76,6 +76,10 @@ enum class st { * * All the statuses except #st::ok should be accompanied with a relevant error message, in case it * propagates back to the main function and is shown to the user. + * + * \todo Instead of being returned, it could be thrown. Most of the error handling code just let the + * status bubble. When we're confident about RAII, we're good to go. When we migrate, let's + * start from main and adapt the functions top-down. */ struct status { status(st code = st::ok) : code(code) {} @@ -158,6 +162,10 @@ bool is_opus_stream(const ogg_page& identification_header); * * Call #read_page repeatedly until #status::end_of_stream to consume the stream, and use #page to * check its content. + * + * \todo This class could be made more intuitive if it acted like an iterator, to be used like + * `for (ogg_page& page : ogg_reader(input))`, but the prerequisite for this is the ability to + * throw an exception on error. */ struct ogg_reader { /** @@ -179,7 +187,7 @@ struct ogg_reader { * * After the last page was read, return #status::end_of_stream. */ - status read_page(); + status next_page(); /** * Read the single packet contained in the last page read, assuming it's a header page, and * call the function f on it. This function has no side effect, and calling it twice on the @@ -188,7 +196,7 @@ struct ogg_reader { * It is currently limited to packets that fit on a single page, and should be later * extended to support packets spanning multiple pages. */ - status read_header_packet(const std::function& f); + status process_header_packet(const std::function& f); /** * Current page from the sync state. * diff --git a/t/ogg.cc b/t/ogg.cc index d1ac483..f4b6181 100644 --- a/t/ogg.cc +++ b/t/ogg.cc @@ -11,12 +11,12 @@ static void check_ref_ogg() ot::ogg_reader reader(input.get()); - ot::status rc = reader.read_page(); + ot::status rc = reader.next_page(); if (rc != ot::st::ok) throw failure("could not read the first page"); if (!ot::is_opus_stream(reader.page)) throw failure("failed to identify the stream as opus"); - rc = reader.read_header_packet([](ogg_packet& p) { + rc = reader.process_header_packet([](ogg_packet& p) { if (p.bytes != 19) throw failure("unexpected length for the first packet"); return ot::st::ok; @@ -24,10 +24,10 @@ static void check_ref_ogg() if (rc != ot::st::ok) throw failure("could not read the first packet"); - rc = reader.read_page(); + rc = reader.next_page(); if (rc != ot::st::ok) throw failure("could not read the second page"); - rc = reader.read_header_packet([](ogg_packet& p) { + rc = reader.process_header_packet([](ogg_packet& p) { if (p.bytes != 62) throw failure("unexpected length for the second packet"); return ot::st::ok; @@ -36,11 +36,11 @@ static void check_ref_ogg() throw failure("could not read the second packet"); while (!ogg_page_eos(&reader.page)) { - rc = reader.read_page(); + rc = reader.next_page(); if (rc != ot::st::ok) throw failure("failure reading a page"); } - rc = reader.read_page(); + rc = reader.next_page(); if (rc != ot::st::end_of_stream) throw failure("did not correctly detect the end of stream"); } @@ -90,27 +90,27 @@ static void check_memory_ogg() if (input == nullptr) throw failure("could not open the input stream"); ot::ogg_reader reader(input.get()); - rc = reader.read_page(); + rc = reader.next_page(); if (rc != ot::st::ok) throw failure("could not read the first page"); - rc = reader.read_header_packet([&first_packet](ogg_packet &p) { + rc = reader.process_header_packet([&first_packet](ogg_packet &p) { if (!same_packet(p, first_packet)) throw failure("unexpected content in the first packet"); return ot::st::ok; }); if (rc != ot::st::ok) throw failure("could not read the first packet"); - rc = reader.read_page(); + rc = reader.next_page(); if (rc != ot::st::ok) throw failure("could not read the second page"); - rc = reader.read_header_packet([&second_packet](ogg_packet &p) { + rc = reader.process_header_packet([&second_packet](ogg_packet &p) { if (!same_packet(p, second_packet)) throw failure("unexpected content in the second packet"); return ot::st::ok; }); if (rc != ot::st::ok) throw failure("could not read the second packet"); - rc = reader.read_page(); + rc = reader.next_page(); if (rc != ot::st::end_of_stream) throw failure("unexpected third page"); }