diff --git a/src/cli.cc b/src/cli.cc index f91ad36..849531d 100644 --- a/src/cli.cc +++ b/src/cli.cc @@ -211,14 +211,14 @@ std::list ot::read_comments(FILE* input) static ot::status process_tags(const ogg_packet& packet, const ot::options& opt, ot::ogg_writer* writer) { ot::opus_tags tags; - if(ot::parse_tags((char*) packet.packet, packet.bytes, &tags) != ot::status::ok) + if(ot::parse_tags(packet, tags) != ot::status::ok) return ot::status::bad_comment_header; if (opt.delete_all) { tags.comments.clear(); } else { for (const std::string& name : opt.to_delete) - ot::delete_tags(&tags, name.c_str()); + ot::delete_comments(tags, name.c_str()); } if (opt.set_all) diff --git a/src/opus.cc b/src/opus.cc index 5f35aad..d795202 100644 --- a/src/opus.cc +++ b/src/opus.cc @@ -46,16 +46,24 @@ ot::status ot::validate_identification_header(const ogg_packet& packet) return ot::status::ok; } -ot::status ot::parse_tags(const char *data, long len, opus_tags *tags) +/** + * \todo See if the packet's data could be casted more nicely into a string. + */ +ot::status ot::parse_tags(const ogg_packet& packet, opus_tags& tags) { - if (len < 0) + if (packet.bytes < 0) return status::int_overflow; - size_t size = static_cast(len); + size_t size = static_cast(packet.bytes); + const char* data = reinterpret_cast(packet.packet); size_t pos = 0; + opus_tags my_tags; + + // Magic number if (8 > size) return status::overflowing_magic_number; if (memcmp(data, "OpusTags", 8) != 0) return status::bad_magic_number; + // Vendor pos = 8; if (pos + 4 > size) @@ -63,14 +71,16 @@ ot::status ot::parse_tags(const char *data, long len, opus_tags *tags) size_t vendor_length = le32toh(*((uint32_t*) (data + pos))); if (pos + 4 + vendor_length > size) return status::overflowing_vendor_data; - tags->vendor = std::string(data + pos + 4, vendor_length); - pos += 4 + tags->vendor.size(); - // Count + my_tags.vendor = std::string(data + pos + 4, vendor_length); + pos += 4 + my_tags.vendor.size(); + + // Comment count if (pos + 4 > size) return status::overflowing_comment_count; uint32_t count = le32toh(*((uint32_t*) (data + pos))); pos += 4; - // Comments + + // Comments' data for (uint32_t i = 0; i < count; ++i) { if (pos + 4 > size) return status::overflowing_comment_length; @@ -78,11 +88,14 @@ ot::status ot::parse_tags(const char *data, long len, opus_tags *tags) if (pos + 4 + comment_length > size) return status::overflowing_comment_data; const char *comment_value = data + pos + 4; - tags->comments.emplace_back(comment_value, comment_length); + my_tags.comments.emplace_back(comment_value, comment_length); pos += 4 + comment_length; } + // Extra data - tags->extra_data = std::string(data + pos, size - pos); + my_tags.extra_data = std::string(data + pos, size - pos); + + tags = std::move(my_tags); return status::ok; } @@ -136,12 +149,12 @@ static int match_field(const char *comment, uint32_t len, const char *field) return 1; } -void ot::delete_tags(opus_tags *tags, const char *field) +void ot::delete_comments(opus_tags& tags, const char* field_name) { - auto it = tags->comments.begin(), end = tags->comments.end(); + auto it = tags.comments.begin(), end = tags.comments.end(); while (it != end) { auto current = it++; - if (match_field(current->data(), current->size(), field)) - tags->comments.erase(current); + if (match_field(current->data(), current->size(), field_name)) + tags.comments.erase(current); } } diff --git a/src/opustags.h b/src/opustags.h index 93c843c..377b3c1 100644 --- a/src/opustags.h +++ b/src/opustags.h @@ -301,9 +301,22 @@ struct opus_tags { */ status validate_identification_header(const ogg_packet& packet); -status parse_tags(const char *data, long len, opus_tags *tags); +/** + * Read the given OpusTags packet and extract its content into an opus_tags object. + * + * On error, the tags object is left unchanged. + */ +status parse_tags(const ogg_packet& packet, opus_tags& tags); + +/** + * Serialize an #opus_tags object into an OpusTags Ogg packet. + */ dynamic_ogg_packet render_tags(const opus_tags& tags); -void delete_tags(opus_tags *tags, const char *field); + +/** + * Remove all the comments whose field name is equal to the special one, case-sensitive. + */ +void delete_comments(opus_tags& tags, const char* field_name); /** \} */ diff --git a/t/opus.cc b/t/opus.cc index 83e60b0..1a27c4a 100644 --- a/t/opus.cc +++ b/t/opus.cc @@ -41,7 +41,10 @@ static const char standard_OpusTags[] = static void parse_standard() { ot::opus_tags tags; - auto rc = ot::parse_tags(standard_OpusTags, sizeof(standard_OpusTags) - 1, &tags); + ogg_packet op; + op.bytes = sizeof(standard_OpusTags) - 1; + op.packet = (unsigned char*) standard_OpusTags; + auto rc = ot::parse_tags(op, tags); if (rc != ot::status::ok) throw failure("ot::parse_tags did not return ok"); if (tags.vendor != "opustags test packet") @@ -69,6 +72,9 @@ static void parse_corrupted() char packet[size]; memcpy(packet, standard_OpusTags, size); ot::opus_tags tags; + ogg_packet op; + op.packet = (unsigned char*) packet; + op.bytes = size; char* header_data = packet; char* vendor_length = header_data + 8; @@ -78,36 +84,42 @@ static void parse_corrupted() char* first_comment_data = first_comment_length + 4; char* end = packet + size; - if (ot::parse_tags(packet, 7, &tags) != ot::status::overflowing_magic_number) + op.bytes = 7; + if (ot::parse_tags(op, tags) != ot::status::overflowing_magic_number) throw failure("did not detect the overflowing magic number"); - if (ot::parse_tags(packet, 11, &tags) != ot::status::overflowing_vendor_length) + op.bytes = 11; + if (ot::parse_tags(op, tags) != ot::status::overflowing_vendor_length) throw failure("did not detect the overflowing vendor string length"); + op.bytes = size; header_data[0] = 'o'; - if (ot::parse_tags(packet, size, &tags) != ot::status::bad_magic_number) + if (ot::parse_tags(op, tags) != ot::status::bad_magic_number) throw failure("did not detect the bad magic number"); header_data[0] = 'O'; *vendor_length = end - vendor_string + 1; - if (ot::parse_tags(packet, size, &tags) != ot::status::overflowing_vendor_data) + if (ot::parse_tags(op, tags) != ot::status::overflowing_vendor_data) throw failure("did not detect the overflowing vendor string"); *vendor_length = end - vendor_string - 3; - if (ot::parse_tags(packet, size, &tags) != ot::status::overflowing_comment_count) + if (ot::parse_tags(op, tags) != ot::status::overflowing_comment_count) throw failure("did not detect the overflowing comment count"); *vendor_length = comment_count - vendor_string; ++*comment_count; - if (ot::parse_tags(packet, size, &tags) != ot::status::overflowing_comment_length) + if (ot::parse_tags(op, tags) != ot::status::overflowing_comment_length) throw failure("did not detect the overflowing comment length"); *first_comment_length = end - first_comment_data + 1; - if (ot::parse_tags(packet, size, &tags) != ot::status::overflowing_comment_data) + if (ot::parse_tags(op, tags) != ot::status::overflowing_comment_data) throw failure("did not detect the overflowing comment data"); } static void recode_standard() { ot::opus_tags tags; - auto rc = ot::parse_tags(standard_OpusTags, sizeof(standard_OpusTags) - 1, &tags); + ogg_packet op; + op.bytes = sizeof(standard_OpusTags) - 1; + op.packet = (unsigned char*) standard_OpusTags; + auto rc = ot::parse_tags(op, tags); if (rc != ot::status::ok) throw failure("ot::parse_tags did not return ok"); auto packet = ot::render_tags(tags); @@ -131,7 +143,11 @@ static void recode_padding() std::string padded_OpusTags(standard_OpusTags, sizeof(standard_OpusTags)); // ^ note: padded_OpusTags ends with a null byte here padded_OpusTags += "hello"; - auto rc = ot::parse_tags(padded_OpusTags.data(), padded_OpusTags.size(), &tags); + ogg_packet op; + op.bytes = padded_OpusTags.size(); + op.packet = (unsigned char*) padded_OpusTags.data(); + + auto rc = ot::parse_tags(op, tags); if (rc != ot::status::ok) throw failure("ot::parse_tags did not return ok"); if (tags.extra_data != "\0hello"s)