Implement small TODOs for v8

This commit is contained in:
Martchus 2018-07-10 17:07:34 +02:00
parent a87ad5f5ec
commit e81c6bb169
15 changed files with 60 additions and 79 deletions

View File

@ -172,9 +172,8 @@ void FlacStream::internalParseHeader(Diagnostics &diag)
* - Padding is skipped
*
* \returns Returns the start offset of the last "METADATA_BLOCK_HEADER" within \a outputStream.
* \todo Return as std::streamoff in v8 to avoid conversion.
*/
uint32 FlacStream::makeHeader(ostream &outputStream, Diagnostics &diag)
std::streamoff FlacStream::makeHeader(ostream &outputStream, Diagnostics &diag)
{
istream &originalStream = m_mediaFileInfo.stream();
originalStream.seekg(static_cast<streamoff>(m_startOffset + 4));
@ -222,7 +221,7 @@ uint32 FlacStream::makeHeader(ostream &outputStream, Diagnostics &diag)
// write Vorbis comment
if (!m_vorbisComment) {
return lastStartOffset >= 0 ? static_cast<uint32>(lastStartOffset) : 0;
return lastStartOffset >= 0 ? lastStartOffset : 0;
}
// leave 4 bytes space for the "METADATA_BLOCK_HEADER"
lastStartOffset = outputStream.tellp();
@ -251,7 +250,7 @@ uint32 FlacStream::makeHeader(ostream &outputStream, Diagnostics &diag)
// write cover fields separately as "METADATA_BLOCK_PICTURE"
if (header.isLast()) {
return static_cast<uint32>(lastStartOffset);
return lastStartOffset;
}
header.setType(FlacMetaDataBlockType::Picture);
const auto coverFields = m_vorbisComment->fields().equal_range(coverId);
@ -289,7 +288,7 @@ uint32 FlacStream::makeHeader(ostream &outputStream, Diagnostics &diag)
outputStream.seekp(lastActuallyWrittenHeader.dataSize());
}
return static_cast<uint32>(lastStartOffset);
return lastStartOffset;
}
/*!

View File

@ -23,7 +23,7 @@ public:
uint32 paddingSize() const;
uint32 streamOffset() const;
uint32 makeHeader(std::ostream &stream, Diagnostics &diag);
std::streamoff makeHeader(std::ostream &stream, Diagnostics &diag);
static void makePadding(std::ostream &stream, uint32 size, bool isLast, Diagnostics &diag);
protected:

View File

@ -396,13 +396,10 @@ void Id3v2Tag::setVersion(byte majorVersion, byte revisionVersion)
/*!
* \brief Returns true if \a lhs goes before \a rhs; otherwise returns false.
* \todo Don't pass args by reference in v8.
* \todo Handle case when Id3v2FrameIds::convertToLongId() returns 0.
*/
bool FrameComparer::operator()(const uint32 &lhsRef, const uint32 &rhsRef) const
bool FrameComparer::operator()(uint32 lhs, uint32 rhs) const
{
uint32 lhs(lhsRef);
uint32 rhs(rhsRef);
if (lhs == rhs) {
return false;
}

View File

@ -12,7 +12,7 @@ namespace TagParser {
class Id3v2Tag;
struct TAG_PARSER_EXPORT FrameComparer {
bool operator()(const uint32 &lhs, const uint32 &rhs) const;
bool operator()(uint32 lhs, uint32 rhs) const;
};
class TAG_PARSER_EXPORT Id3v2TagMaker {

View File

@ -135,35 +135,6 @@ void MatroskaTag::parse(EbmlElement &tagElement, Diagnostics &diag)
}
}
/*!
* \brief Prepares making.
* \returns Returns a MatroskaTagMaker object which can be used to actually make the tag.
* \remarks The tag must NOT be mutated after making is prepared when it is intended to actually
* make the tag using the make() method of the returned object.
* \throws Throws TagParser::Failure or a derived exception when a making error occurs.
*
* This method might be useful when it is necessary to know the size of the tag before making it.
* \sa make()
* \todo Make inline in next major release.
*/
MatroskaTagMaker MatroskaTag::prepareMaking(Diagnostics &diag)
{
return MatroskaTagMaker(*this, diag);
}
/*!
* \brief Writes tag information to the specified \a stream (makes a "Tag"-element).
* \throws Throws std::ios_base::failure when an IO error occurs.
* \throws Throws TagParser::Failure or a derived exception when a making
* error occurs.
* \sa prepareMaking()
* \todo Make inline in next major release.
*/
void MatroskaTag::make(ostream &stream, Diagnostics &diag)
{
prepareMaking(diag).make(stream);
}
/*!
* \brief Parses the specified \a targetsElement.
*

View File

@ -97,6 +97,33 @@ inline TagTargetLevel MatroskaTag::targetLevel() const
return matroskaTagTargetLevel(m_target.level());
}
/*!
* \brief Prepares making.
* \returns Returns a MatroskaTagMaker object which can be used to actually make the tag.
* \remarks The tag must NOT be mutated after making is prepared when it is intended to actually
* make the tag using the make() method of the returned object.
* \throws Throws TagParser::Failure or a derived exception when a making error occurs.
*
* This method might be useful when it is necessary to know the size of the tag before making it.
* \sa make()
*/
inline MatroskaTagMaker MatroskaTag::prepareMaking(Diagnostics &diag)
{
return MatroskaTagMaker(*this, diag);
}
/*!
* \brief Writes tag information to the specified \a stream (makes a "Tag"-element).
* \throws Throws std::ios_base::failure when an IO error occurs.
* \throws Throws TagParser::Failure or a derived exception when a making
* error occurs.
* \sa prepareMaking()
*/
inline void MatroskaTag::make(std::ostream &stream, Diagnostics &diag)
{
prepareMaking(diag).make(stream);
}
inline bool MatroskaTag::canEncodingBeUsed(TagTextEncoding encoding) const
{
return encoding == TagTextEncoding::Utf8;

View File

@ -80,7 +80,6 @@ private:
* This method might be useful when it is necessary to know the size of the track header before making
* it.
* \sa make()
* \todo Make inline in next major release.
*/
inline MatroskaTrackHeaderMaker MatroskaTrack::prepareMakingHeader(Diagnostics &diag) const
{
@ -93,7 +92,6 @@ inline MatroskaTrackHeaderMaker MatroskaTrack::prepareMakingHeader(Diagnostics &
* \throws Throws TagParser::Failure or a derived exception when a making
* error occurs.
* \sa prepareMaking()
* \todo Make inline in next major release.
*/
inline void MatroskaTrack::makeHeader(std::ostream &stream, Diagnostics &diag) const
{

View File

@ -1046,40 +1046,38 @@ Id3v2Tag *MediaFileInfo::createId3v2Tag()
* \remarks Invalidates the removed tag object if it has been removed.
*
* \sa applyChanges()
* \todo Make this return whether the \a tag could be removed in v8.
*/
void MediaFileInfo::removeTag(Tag *tag)
bool MediaFileInfo::removeTag(Tag *tag)
{
if (!tag) {
return;
return false;
}
// remove tag via container
if (m_container) {
m_container->removeTag(tag);
return;
return m_container->removeTag(tag);
}
// remove tag via track for "single-track" formats
if (m_singleTrack && m_containerFormat == ContainerFormat::Flac) {
auto *const flacStream(static_cast<FlacStream *>(m_singleTrack.get()));
if (flacStream->vorbisComment() == tag) {
flacStream->removeVorbisComment();
return;
return flacStream->removeVorbisComment();
}
}
// remove ID3 tags
if (m_id3v1Tag.get() == tag) {
m_id3v1Tag.reset();
return;
return true;
}
for (auto i = m_id3v2Tags.begin(), end = m_id3v2Tags.end(); i != end; ++i) {
if (i->get() == tag) {
m_id3v2Tags.erase(i);
break;
return true;
}
}
return false;
}
/*!
@ -1550,11 +1548,11 @@ void MediaFileInfo::makeMp3File(Diagnostics &diag, AbortableProgressFeedback &pr
uint32 streamOffset; // where the actual stream starts
stringstream flacMetaData(ios_base::in | ios_base::out | ios_base::binary);
flacMetaData.exceptions(ios_base::badbit | ios_base::failbit);
uint32 startOfLastMetaDataBlock;
std::streamoff startOfLastMetaDataBlock;
if (flacStream) {
// if it is a raw FLAC stream, make FLAC metadata
startOfLastMetaDataBlock = flacStream->makeHeader(flacMetaData, diag);
tagsSize += static_cast<uint32>(flacMetaData.tellp());
tagsSize += flacMetaData.tellp();
streamOffset = flacStream->streamOffset();
} else {
// make no further metadata, just use the container offset as stream offset
@ -1664,8 +1662,8 @@ void MediaFileInfo::makeMp3File(Diagnostics &diag, AbortableProgressFeedback &pr
if (flacStream) {
if (padding && startOfLastMetaDataBlock) {
// if appending padding, ensure the last flag of the last "METADATA_BLOCK_HEADER" is not set
flacMetaData.seekg(static_cast<streamoff>(startOfLastMetaDataBlock));
flacMetaData.seekp(static_cast<streamoff>(startOfLastMetaDataBlock));
flacMetaData.seekg(startOfLastMetaDataBlock);
flacMetaData.seekp(startOfLastMetaDataBlock);
flacMetaData.put(static_cast<byte>(flacMetaData.peek()) & (0x80u - 1));
flacMetaData.seekg(0);
}

View File

@ -110,7 +110,7 @@ public:
bool removeId3v2Tag(Id3v2Tag *tag);
bool removeAllId3v2Tags();
Id3v2Tag *createId3v2Tag();
void removeTag(Tag *tag);
bool removeTag(Tag *tag);
void removeAllTags();
void mergeId3v2Tags();
bool id3v1ToId3v2();

View File

@ -126,17 +126,17 @@ void Mp4Atom::internalParse(Diagnostics &diag)
* \throw The caller has to be sure, that the number of written bytes does not exceed
* maximum of an 32-bit unsigned integer. Otherwise the function will throw
* Failure and Mp4Atom::seekBackAndWriteAtomSize64 should be used instead.
* \todo Add a version which creates a diag message (in addition to throwing an exception).
*
* This function seeks back to the start offset and writes the difference between the
* previous offset and the start offset as 32-bit unsigned integer to the \a stream.
* Then it seeks back to the previous offset.
*/
void Mp4Atom::seekBackAndWriteAtomSize(std::ostream &stream, const ostream::pos_type &startOffset)
void Mp4Atom::seekBackAndWriteAtomSize(std::ostream &stream, const ostream::pos_type &startOffset, Diagnostics &diag)
{
ostream::pos_type currentOffset = stream.tellp();
const auto atomSize(currentOffset - startOffset);
if (atomSize > numeric_limits<uint32>::max()) {
diag.emplace_back(DiagLevel::Fatal, argsToString(atomSize, " exceeds maximum."), "write 32-bit atom size");
throw Failure();
}
stream.seekp(startOffset);

View File

@ -46,7 +46,7 @@ public:
bool isPadding() const;
uint64 firstChildOffset() const;
static void seekBackAndWriteAtomSize(std::ostream &stream, const std::ostream::pos_type &startOffset);
static void seekBackAndWriteAtomSize(std::ostream &stream, const std::ostream::pos_type &startOffset, Diagnostics &diag);
static void seekBackAndWriteAtomSize64(std::ostream &stream, const std::ostream::pos_type &startOffset);
static constexpr void addHeaderSize(uint64 &dataSize);
static void makeHeader(uint64 size, uint32 id, IoUtilities::BinaryWriter &writer);

View File

@ -1159,7 +1159,7 @@ void Mp4Track::makeTrack(Diagnostics &diag)
// write mdia atom
makeMedia(diag);
// write size (of trak atom)
Mp4Atom::seekBackAndWriteAtomSize(outputStream(), trakStartOffset);
Mp4Atom::seekBackAndWriteAtomSize(outputStream(), trakStartOffset, diag);
}
/*!
@ -1307,7 +1307,7 @@ void Mp4Track::makeMedia(Diagnostics &diag)
// write minf atom
makeMediaInfo(diag);
// write size (of mdia atom)
Mp4Atom::seekBackAndWriteAtomSize(outputStream(), mdiaStartOffset);
Mp4Atom::seekBackAndWriteAtomSize(outputStream(), mdiaStartOffset, diag);
}
/*!
@ -1372,7 +1372,7 @@ void Mp4Track::makeMediaInfo(Diagnostics &diag)
"Source track does not contain mandatory stbl atom and the tagparser lib is unable to make one from scratch.", "making stbl atom");
}
// write size (of minf atom)
Mp4Atom::seekBackAndWriteAtomSize(outputStream(), minfStartOffset);
Mp4Atom::seekBackAndWriteAtomSize(outputStream(), minfStartOffset, diag);
}
/*!
@ -1436,7 +1436,7 @@ void Mp4Track::makeSampleTable(Diagnostics &diag)
// write subs atom (sub-sample information)
// write size (of stbl atom)
Mp4Atom::seekBackAndWriteAtomSize(outputStream(), stblStartOffset);
Mp4Atom::seekBackAndWriteAtomSize(outputStream(), stblStartOffset, diag);
}
void Mp4Track::internalParseHeader(Diagnostics &diag)

View File

@ -35,7 +35,7 @@ public:
uint32 currentSegmentSize() const;
void setFilter(uint32 streamSerialId);
void removeFilter();
bool areAllPagesFetched() const;
bool isLastPageFetched() const;
void read(char *buffer, std::size_t count);
size_t readAll(char *buffer, std::size_t max);
void ignore(std::size_t count = 1);
@ -255,17 +255,9 @@ inline void OggIterator::removeFilter()
}
/*!
* \brief Returns an indication whether all pages have been fetched.
*
* This means that for each page in the stream in the specified range (stream and range have been specified when
* constructing the iterator) an OggPage instance has been created and pushed to pages(). This is independend from
* the current iterator position. Fetched pages remain after resetting the iterator.
*
* \remarks This is also true if pages in the middle of the file have been omitted because it is actually just checked
* whether the last page has been fetched.
* \todo Rename to isLastPageFetched() in next major release.
* \brief Returns whether the last page has already been fetched.
*/
inline bool OggIterator::areAllPagesFetched() const
inline bool OggIterator::isLastPageFetched() const
{
return (m_pages.empty() ? m_startOffset : m_pages.back().startOffset() + m_pages.back().totalSize()) >= m_streamSize;
}

View File

@ -293,7 +293,7 @@ void OggStream::calculateDurationViaSampleCount(uint16 preSkip)
// determine sample count
const auto &iterator = m_container.m_iterator;
if (!m_sampleCount && iterator.areAllPagesFetched()) {
if (!m_sampleCount && iterator.isLastPageFetched()) {
const auto &pages = iterator.pages();
const auto firstPage = find_if(pages.cbegin(), pages.cend(), pred);
const auto lastPage = find_if(pages.crbegin(), pages.crend(), pred);

View File

@ -75,7 +75,7 @@ public:
TagValue(const char *data, std::size_t length, TagDataType type = TagDataType::Undefined, TagTextEncoding encoding = TagTextEncoding::Latin1);
TagValue(std::unique_ptr<char[]> &&data, std::size_t length, TagDataType type = TagDataType::Binary,
TagTextEncoding encoding = TagTextEncoding::Latin1);
TagValue(const PositionInSet &value);
TagValue(PositionInSet value);
TagValue(const TagValue &other);
TagValue(TagValue &&other) = default;
~TagValue();
@ -278,9 +278,8 @@ inline TagValue::TagValue(std::unique_ptr<char[]> &&data, std::size_t length, Ta
/*!
* \brief Constructs a new TagValue holding a copy of the given PositionInSet \a value.
* \param value Specifies the PositionInSet.
* \todo Pass \a value by value in v8.
*/
inline TagValue::TagValue(const PositionInSet &value)
inline TagValue::TagValue(PositionInSet value)
: TagValue(reinterpret_cast<const char *>(&value), sizeof(value), TagDataType::PositionInSet)
{
}