diff --git a/src/cli.cc b/src/cli.cc index a651b81..b00362e 100644 --- a/src/cli.cc +++ b/src/cli.cc @@ -226,15 +226,13 @@ static ot::status process_tags(const ogg_packet& packet, const ot::options& opt, for (const std::string& comment : opt.to_add) tags.comments.emplace_back(comment); - if (writer.file) { + if (writer) { auto packet = ot::render_tags(tags); - if(ogg_stream_packetin(&writer.stream, &packet) == -1) - return ot::status::libogg_error; + return writer.write_packet(packet); } else { ot::print_comments(tags.comments, stdout); + return ot::status::ok; } - - return ot::status::ok; } /** @@ -244,7 +242,7 @@ static ot::status process_tags(const ogg_packet& packet, const ot::options& opt, ot::status ot::process(ogg_reader& reader, ogg_writer& writer, const ot::options &opt) { const char *error = nullptr; - int packet_count = -1; + int packet_count = 0; while (error == nullptr) { // Read the next page. ot::status rc = reader.read_page(); @@ -258,22 +256,16 @@ ot::status ot::process(ogg_reader& reader, ogg_writer& writer, const ot::options break; } // Short-circuit when the relevant packets have been read. - if (packet_count >= 2 && writer.file) { - if (ot::write_page(&reader.page, writer.file) == -1) { - error = "write_page: fwrite error"; + if (packet_count >= 2 && writer) { + if (writer.write_page(reader.page) != ot::status::ok) { + error = "error writing ogg page"; break; } continue; } - // Initialize the streams from the first page. - if (packet_count == -1) { - if (writer.file) { - if (ogg_stream_init(&writer.stream, ogg_page_serialno(&reader.page)) == -1) { - error = "ogg_stream_init: couldn't create an encoder"; - break; - } - } - packet_count = 0; + if (writer && writer.prepare_stream(ogg_page_serialno(&reader.page)) != ot::status::ok) { + error = "ogg_stream_init: could not prepare the ogg stream"; + break; } // Read all the packets. while ((rc = reader.read_packet()) == ot::status::ok) { @@ -290,30 +282,24 @@ ot::status ot::process(ogg_reader& reader, ogg_writer& writer, const ot::options error = ot::error_message(rc); break; } - if (!writer.file) + if (!writer) break; /* nothing else to do */ else continue; /* process_tags wrote the new packet */ } - if (writer.file) { - if (ogg_stream_packetin(&writer.stream, &reader.packet) == -1) { - error = "ogg_stream_packetin: internal error"; - break; - } + if (writer && writer.write_packet(reader.packet) != ot::status::ok) { + error = "error feeding the packet to the ogg stream"; + break; } } if (rc != ot::status::ok && rc != ot::status::end_of_page) { error = "error reading the ogg packets"; break; } - // Write the page. - if (writer.file) { - ogg_page page; - ogg_stream_flush(&writer.stream, &page); - if (ot::write_page(&page, writer.file) == -1) - error = "write_page: fwrite error"; - else if (ogg_stream_check(&writer.stream) != 0) - error = "ogg_stream_check: internal error (encoder)"; + // Write the assembled page. + if (writer && writer.flush_page() != ot::status::ok) { + error = "error flushing the ogg page"; + break; } } if (!error && packet_count < 2) diff --git a/src/ogg.cc b/src/ogg.cc index 3843f05..8e4e313 100644 --- a/src/ogg.cc +++ b/src/ogg.cc @@ -62,7 +62,8 @@ ot::status ot::ogg_reader::read_packet() ot::ogg_writer::ogg_writer(FILE* output) : file(output) { - memset(&stream, 0, sizeof(stream)); + if (ogg_stream_init(&stream, 0) != 0) + throw std::bad_alloc(); } ot::ogg_writer::~ogg_writer() @@ -70,11 +71,42 @@ ot::ogg_writer::~ogg_writer() ogg_stream_clear(&stream); } -int ot::write_page(ogg_page *og, FILE *stream) +ot::status ot::ogg_writer::write_page(const ogg_page& page) { - if((ssize_t) fwrite(og->header, 1, og->header_len, stream) < og->header_len) - return -1; - if((ssize_t) fwrite(og->body, 1, og->body_len, stream) < og->body_len) - return -1; - return 0; + if (page.header_len < 0 || page.body_len < 0) + return status::int_overflow; + auto header_len = static_cast(page.header_len); + auto body_len = static_cast(page.body_len); + if (fwrite(page.header, 1, header_len, file) < header_len) + return status::standard_error; + if (fwrite(page.body, 1, body_len, file) < body_len) + return status::standard_error; + return status::ok; +} + +ot::status ot::ogg_writer::prepare_stream(long serialno) +{ + if (stream.serialno != serialno) { + if (ogg_stream_reset_serialno(&stream, serialno) != 0) + return status::libogg_error; + } + return status::ok; +} + +ot::status ot::ogg_writer::write_packet(const ogg_packet& packet) +{ + if (ogg_stream_packetin(&stream, const_cast(&packet)) != 0) + return status::libogg_error; + else + return status::ok; +} + +ot::status ot::ogg_writer::flush_page() +{ + ogg_page page; + if (ogg_stream_flush(&stream, &page) != 0) + return write_page(page); + if (ogg_stream_check(&stream) != 0) + return status::libogg_error; + return status::ok; /* nothing was done */ } diff --git a/src/opustags.h b/src/opustags.h index 43af319..da196ad 100644 --- a/src/opustags.h +++ b/src/opustags.h @@ -166,23 +166,70 @@ private: ogg_stream_state stream; }; -struct ogg_writer { +/** + * An Ogg writer lets you write ogg_page objets to an output file, and assemble packets into pages. + * + * It has two modes of operations : + * 1. call #write_page, or + * 2. call #prepare_stream, then #write_packet one or more times, followed by #flush_page. + * + * You can switch between the two modes, but must not start writing packets and then pages without + * flushing. + */ +class ogg_writer { +public: /** - * Zeroes the stream state. You need to initialize it with the serialno. - * - * Initialize #file with the given handle. The caller is responsible for keeping the file - * handle alive, and to close it. + * Initialize the writer with the given output file handle. The caller is responsible for + * keeping the file handle alive, and to close it. */ ogg_writer(FILE* output); /** * Clears the stream state and any internal memory. Does not close the output file. */ ~ogg_writer(); + /** + * Returns true if the writer was open on a non-null file. + * + * \todo We should not create invalid writers instead. + */ + operator bool() const { return file != nullptr; } + /** + * Write a whole Ogg page into the output stream. + * + * This is a basic I/O operation and does not even require libogg, or the stream. + */ + status write_page(const ogg_page& page); + /** + * Prepare the stream with the given Ogg serial number. + * + * If the stream is already configured with the right serial number, it doesn't do anything + * and is cheap to call. + * + * If the stream contains unflushed packets, they will be lost. + */ + status prepare_stream(long serialno); + /** + * Add a packet to the current page under assembly. + * + * If the packet is coming from a different page, make sure the serial number fits by + * calling #prepare_stream. + * + * When the page is complete, you should call #flush_page to finalize the page. + * + * You must not call #write_page after it, until you call #flush_page. + */ + status write_packet(const ogg_packet& packet); + /** + * Write the page under assembly. Future calls to #write_packet will be written in a new + * page. + */ + status flush_page(); +private: /** * The stream state receives packets and generates pages. * - * We only need it to put the OpusHead and OpusTags packets into their own pages. The other - * pages are naively written to the output stream. + * In our specific use case, we only need it to put the OpusHead and OpusTags packets into + * their own pages. The other pages are naively written to the output stream. */ ogg_stream_state stream; /** @@ -192,8 +239,6 @@ struct ogg_writer { FILE* file; }; -int write_page(ogg_page *og, FILE *stream); - /** * Ogg packet with dynamically allocated data. *