polish ogg_reader

src/cli.cc

@@ -248,7 +248,7 @@ ot::status ot::process(ogg_reader& reader, ogg_writer& writer, const ot::options
 	while (error == nullptr) {
 		// Read the next page.
 		ot::status rc = reader.read_page();
-		if (rc == ot::status::end_of_file) {
+		if (rc == ot::status::end_of_stream) {
 			break;
 		} else if (rc != ot::status::ok) {
 			if (rc == ot::status::standard_error)

src/error.cc

@@ -7,7 +7,7 @@ static const char* messages[] = {
 	"Bad command-line arguments",
 	"Integer overflow",
 	nullptr, /* standard error: call stderror */
-	"End of file",
+	"End of stream",
 	"End of page",
 	"Stream is not ready",
 	"libogg error",

src/ogg.cc

@@ -19,7 +19,7 @@ ot::status ot::ogg_reader::read_page()
 {
 	while (ogg_sync_pageout(&sync, &page) != 1) {
 		if (feof(file))
-			return status::end_of_file;
+			return status::end_of_stream;
 		char* buf = ogg_sync_buffer(&sync, 65536);
 		if (buf == nullptr)
 			return status::libogg_error;
@@ -31,8 +31,9 @@ ot::status ot::ogg_reader::read_page()
 		if (ogg_sync_check(&sync) != 0)
 			return status::libogg_error;
 	}
+	/* at this point, we've got a good page */
 	if (!stream_ready) {
-		if (ogg_stream_init(&stream, ogg_page_serialno(&page)) == -1)
+		if (ogg_stream_init(&stream, ogg_page_serialno(&page)) != 0)
 			return status::libogg_error;
 		stream_ready = true;
 	}
@@ -44,16 +45,18 @@ ot::status ot::ogg_reader::read_packet()
 {
 	if (!stream_ready)
 		return status::stream_not_ready;
-	/* If the stream is on a different page, feed the current page. */
 	if (!stream_in_sync) {
-		if (ogg_stream_pagein(&stream, &page) == -1)
+		if (ogg_stream_pagein(&stream, &page) != 0)
 			return status::libogg_error;
 		stream_in_sync = true;
 	}
 	int rc = ogg_stream_packetout(&stream, &packet);
-	return rc == 0 ? status::end_of_page :
-	       rc == 1 ? status::ok :
-	                 status::libogg_error;
+	if (rc == 1)
+		return status::ok;
+	else if (rc == 0 && ogg_stream_check(&stream) == 0)
+		return status::end_of_page;
+	else
+		return status::libogg_error;
 }
 
 ot::ogg_writer::ogg_writer(FILE* output)

src/opustags.h

@@ -34,7 +34,7 @@ enum class status {
 	int_overflow,
 	/** On standard error, errno will give more details. */
 	standard_error,
-	end_of_file,
+	end_of_stream,
 	end_of_page,
 	stream_not_ready,
 	libogg_error,
@@ -72,13 +72,18 @@ const char* error_message(status code);
  * \{
  */
 
-struct ogg_reader {
+/**
+ * Ogg reader, combining a FILE input, an ogg_sync_state reading the pages, and an ogg_stream_state
+ * extracting the packets from the page.
+ *
+ * Call #read_page repeatedly until #status::end_of_stream to consume the stream, and use #page to
+ * check its content. To extract its packets, call #read_packet until #status::end_of_packet.
+ */
+class ogg_reader {
+public:
 	/**
-	 * Initialize the sync state and zero-initialize the stream. You'll need to initialize the
-	 * stream yourself once you have the serialno.
-	 *
-	 * Initialize #file with the given handle. The caller is responsible for keeping the file
-	 * handle alive, and to close it.
+	 * Initialize the reader with the given input file handle. The caller is responsible for
+	 * keeping the file handle alive, and to close it.
 	 */
 	ogg_reader(FILE* input);
 	/**
@@ -90,22 +95,42 @@ struct ogg_reader {
 	~ogg_reader();
 	/**
 	 * Read the next page from the input file. The result, provided the status is #status::ok,
-	 * is available in the #page field, is owned by the Ogg reader, and is valid until the next
-	 * call to #read_page. Make sure you also check #status::end_of_file.
+	 * is made available in the #page field, is owned by the Ogg reader, and is valid until the
+	 * next call to #read_page.
+	 *
+	 * After the last page was read, return #status::end_of_stream.
 	 */
 	status read_page();
 	/**
-	 * Read the next available packet from the current #page. The packet is available in the
-	 * #packet field.
+	 * Read the next available packet from the current #page. The packet is made available in
+	 * the #packet field.
 	 *
-	 * If the end of page was reached, return #status::end_of_page.
+	 * No packet can be read until a page has been loaded with #read_page. If that happens,
+	 * return #status::stream_not_ready.
+	 *
+	 * After the last packet was read, return #status::end_of_page.
 	 */
 	status read_packet();
+	/**
+	 * Current page from the sync state.
+	 *
+	 * Its memory is managed by libogg, inside the sync state, and is valid until the next call
+	 * to ogg_sync_pageout, wrapped by #read_page.
+	 */
+	ogg_page page;
+	/**
+	 * Current packet from the stream state.
+	 *
+	 * Its memory is managed by libogg, inside the stream state, and is valid until the next
+	 * call to ogg_stream_packetout, wrapped by #read_packet.
+	 */
+	ogg_packet packet;
+private:
 	/**
 	 * The file is our source of binary data. It is not integrated to libogg, so we need to
 	 * handle it ourselves.
 	 *
-	 * The file is not owned by the reader, you need to close it yourself when you're done.
+	 * The file is not owned by the ogg_reader instance.
 	 */
 	FILE* file;
 	/**
@@ -116,27 +141,17 @@ struct ogg_reader {
 	 * are simply forwarded to the Ogg writer.
 	 */
 	ogg_sync_state sync;
-	/**
-	 * Current page from the sync state.
-	 *
-	 * Its memory is managed by libogg, inside the sync state, and is valid until the next call
-	 * to ogg_sync_pageout.
-	 */
-	ogg_page page;
-	/**
-	 * Current packet from the stream state.
-	 *
-	 * Its memory is managed by libogg, inside the stream state, and is valid until the next
-	 * call to ogg_stream_packetout.
-	 */
-	ogg_packet packet;
-private:
 	/**
 	 * Indicates whether the stream has been initialized or not.
+	 *
+	 * To initialize it properly, we need the serialno of the stream, which is available only
+	 * after the first page was read.
 	 */
 	bool stream_ready = false;
 	/**
 	 * Indicates if the stream's last fed page is the current one.
+	 *
+	 * Its state is irrelevant if the stream is not ready.
 	 */
 	bool stream_in_sync;
 	/**