From: Seungbae Shin Date: Mon, 11 Jul 2022 12:39:16 +0000 (+0900) Subject: Additional refactoring X-Git-Tag: submit/tizen/20220718.050845~1 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=refs%2Fchanges%2F50%2F277650%2F13;p=platform%2Fcore%2Fapi%2Fmetadata-editor.git Additional refactoring - apply factory method pattern - make const member function whenever possible - use member-initializer instead of constructor-initialization - remove some duplicated codes - add some empty lines for enhancing the readability - remove namespace which is already using Change-Id: I5ee8d1e4216803011de158d69a7e2a9b88f51438 --- diff --git a/src/metadata_editor.cpp b/src/metadata_editor.cpp index 81f57e7..42dac80 100755 --- a/src/metadata_editor.cpp +++ b/src/metadata_editor.cpp @@ -48,39 +48,44 @@ static String __get_file_ext(const char *file_path) return ""; } -static const String __get_picture_type(const char *path) +static const char * __get_picture_type(const char *path) { String ext = __get_file_ext(path); - if (ext == "JPG" || ext == "JPEG") { + if (ext == "JPG" || ext == "JPEG") return MIME_TYPE_JPEG; - } else if (ext == "PNG") { + if (ext == "PNG") return MIME_TYPE_PNG; - } + return ""; } class PictureFrame { public: - explicit PictureFrame(const char *path) : stream(path, true) {} - ~PictureFrame() = default; + explicit PictureFrame(const char *path) + : stream(path, true) {} + virtual ~PictureFrame() = default; + ByteVector data() { return stream.readBlock(stream.length()); } + String mime() const { return __get_picture_type(stream.name()); } - bool opened() { + + bool opened() const { return stream.isOpen(); } - MP4::CoverArt::Format mp4format() { + + MP4::CoverArt::Format mp4format() const { if (mime() == MIME_TYPE_JPEG) return MP4::CoverArt::JPEG; - else if (mime() == MIME_TYPE_PNG) + if (mime() == MIME_TYPE_PNG) return MP4::CoverArt::PNG; - else - return MP4::CoverArt::Unknown; + return MP4::CoverArt::Unknown; } + private: FileStream stream; }; @@ -89,7 +94,8 @@ template static bool __is_valid_index(const List& lst, int index) { ME_RETVM_IF(lst.isEmpty(), false, "No picture"); - ME_RETVM_IF((index < 0) || (lst.size() <= static_cast(index)), false, "Out of range:size[%d] index[%d]", lst.size(), index); + ME_RETVM_IF((index < 0) || (lst.size() <= static_cast(index)), + false, "Out of range:size[%d] index[%d]", lst.size(), index); return true; } @@ -134,6 +140,7 @@ static int __append_APIC(ID3v2::Tag *tag, PictureFrame& pic) auto pictureFrame = new ID3v2::AttachedPictureFrame(); pictureFrame->setPicture(pic.data()); pictureFrame->setMimeType(pic.mime()); + tag->addFrame(pictureFrame); return METADATA_EDITOR_ERROR_NONE; @@ -146,6 +153,7 @@ static int __append_ogg_picture(Ogg::XiphComment *xtag, PictureFrame& pic) auto frontCover = new FLAC::Picture; frontCover->setData(pic.data()); frontCover->setMimeType(pic.mime()); + xtag->addPicture(frontCover); return METADATA_EDITOR_ERROR_NONE; @@ -172,6 +180,7 @@ static int __remove_ogg_picture(Ogg::XiphComment *xtag, int index) List::Iterator it = lst.begin(); std::advance(it, index); + xtag->removePicture(*it, true); return METADATA_EDITOR_ERROR_NONE; @@ -181,6 +190,7 @@ class IAlbumArt { public: virtual ~IAlbumArt() = default; + virtual int append(PictureFrame &pic) = 0; virtual int remove(int index) = 0; virtual int read(int index, void **picture, int *size, char **mime_type) = 0; @@ -189,33 +199,35 @@ public: class Mp3AlbumArt : public IAlbumArt { public: - explicit Mp3AlbumArt(FileRef *fileref) { - file = dynamic_cast(fileref->file()); - } - ~Mp3AlbumArt() override {}; + explicit Mp3AlbumArt(FileRef *fileref) + : file(dynamic_cast(fileref->file())) {} + ~Mp3AlbumArt() override = default; int append(PictureFrame &pic) override { return __append_APIC(file->ID3v2Tag(true), pic); } + int remove(int index) override { return __remove_APIC(file->ID3v2Tag(), index); } + int read(int index, void **picture, int *size, char **mime_type) override { return __get_APIC(file->ID3v2Tag(), index, picture, size, mime_type); } + uint count() override { return file->ID3v2Tag() ? file->ID3v2Tag()->frameListMap()["APIC"].size() : 0; } + private: MPEG::File *file; }; class Mp4AlbumArt : public IAlbumArt { public: - explicit Mp4AlbumArt(FileRef *fileref) { - file = dynamic_cast(fileref->file()); - } - ~Mp4AlbumArt() override {}; + explicit Mp4AlbumArt(FileRef *fileref) + : file(dynamic_cast(fileref->file())) {} + ~Mp4AlbumArt() override = default; int append(PictureFrame &pic) override { MP4::CoverArtList lst; @@ -230,6 +242,7 @@ public: return METADATA_EDITOR_ERROR_NONE; } + int remove(int index) override { ME_RETVM_IF(!(file->tag()->contains("covr")), METADATA_EDITOR_ERROR_INVALID_PARAMETER, "No picture"); auto lst = file->tag()->item("covr").toCoverArtList(); @@ -243,6 +256,7 @@ public: return METADATA_EDITOR_ERROR_NONE; } + int read(int index, void **picture, int *size, char **mime_type) override { ME_RETVM_IF(!(file->tag()->contains("covr")), METADATA_EDITOR_ERROR_INVALID_PARAMETER, "No picture"); auto lst = file->tag()->item("covr").toCoverArtList(); @@ -261,19 +275,20 @@ public: return METADATA_EDITOR_ERROR_NONE; } + uint count() override { return (file->tag() && file->tag()->contains("covr")) ? file->tag()->item("covr").toCoverArtList().size() : 0; } + private: MP4::File *file; }; class FlacAlbumArt : public IAlbumArt { public: - explicit FlacAlbumArt(FileRef *fileref) { - file = dynamic_cast(fileref->file()); - } - ~FlacAlbumArt() override {}; + explicit FlacAlbumArt(FileRef *fileref) + : file(dynamic_cast(fileref->file())) {} + ~FlacAlbumArt() override = default; int append(PictureFrame &pic) override { auto frontCover = new FLAC::Picture; @@ -282,6 +297,7 @@ public: file->addPicture(frontCover); return METADATA_EDITOR_ERROR_NONE; } + int remove(int index) override { auto lst = file->pictureList(); ME_RETV_IF(!__is_valid_index(lst, index), METADATA_EDITOR_ERROR_INVALID_PARAMETER); @@ -289,87 +305,122 @@ public: file->removePicture(lst[index], true); return METADATA_EDITOR_ERROR_NONE; } + int read(int index, void **picture, int *size, char **mime_type) override { return __get_flac_picture(file->pictureList(), index, picture, size, mime_type); } + uint count() override { return file->pictureList().size(); } + private: FLAC::File *file; }; class OggVorbisAlbumArt : public IAlbumArt { public: - explicit OggVorbisAlbumArt(FileRef *fileref) { - file = dynamic_cast(fileref->file()); - } - ~OggVorbisAlbumArt() override {}; + explicit OggVorbisAlbumArt(FileRef *fileref) + : file(dynamic_cast(fileref->file())) {} + ~OggVorbisAlbumArt() override = default; int append(PictureFrame &pic) override { return __append_ogg_picture(file->tag(), pic); } + int remove(int index) override { return __remove_ogg_picture(file->tag(), index); } + int read(int index, void **picture, int *size, char **mime_type) override { ME_RETVM_IF(!file->tag(), METADATA_EDITOR_ERROR_INVALID_PARAMETER, "Invalid XiphComment"); return __get_flac_picture(file->tag()->pictureList(), index, picture, size, mime_type); } + uint count() override { return file->tag() ? file->tag()->pictureList().size() : 0; } + private: Ogg::Vorbis::File *file; }; class OggFlacAlbumArt : public IAlbumArt { public: - explicit OggFlacAlbumArt(FileRef *fileref) { - file = dynamic_cast(fileref->file()); - } - ~OggFlacAlbumArt() override {}; + explicit OggFlacAlbumArt(FileRef *fileref) + : file(dynamic_cast(fileref->file())) {} + ~OggFlacAlbumArt() override = default; int append(PictureFrame &pic) override { return __append_ogg_picture(file->tag(), pic); } + int remove(int index) override { return __remove_ogg_picture(file->tag(), index); } + int read(int index, void **picture, int *size, char **mime_type) override { ME_RETVM_IF(!file->tag(), METADATA_EDITOR_ERROR_INVALID_PARAMETER, "Invalid XiphComment"); return __get_flac_picture(file->tag()->pictureList(), index, picture, size, mime_type); } + uint count() override { return file->tag() ? file->tag()->pictureList().size() : 0; } + private: Ogg::FLAC::File *file; }; class WavAlbumArt : public IAlbumArt { public: - explicit WavAlbumArt(FileRef *fileref) { - file = dynamic_cast(fileref->file()); - } - ~WavAlbumArt() override {}; + explicit WavAlbumArt(FileRef *fileref) + : file(dynamic_cast(fileref->file())) {} + ~WavAlbumArt() override = default; int append(PictureFrame &pic) override { return __append_APIC(file->tag(), pic); } + int remove(int index) override { return __remove_APIC(file->tag(), index); } + int read(int index, void **picture, int *size, char **mime_type) override { return __get_APIC(file->tag(), index, picture, size, mime_type); } + uint count() override { return file->tag() ? file->tag()->frameListMap()["APIC"].size() : 0; } + private: RIFF::WAV::File *file; }; +class AlbumArtFactory { +public: + static IAlbumArt* create(FileRef *fref) { + if (fref == nullptr) + return nullptr; + + if (dynamic_cast(fref->file())) + return new Mp3AlbumArt(fref); + if (dynamic_cast(fref->file())) + return new Mp4AlbumArt(fref); + if (dynamic_cast(fref->file())) + return new FlacAlbumArt(fref); + if (dynamic_cast(fref->file())) + return new WavAlbumArt(fref); + if (dynamic_cast(fref->file())) + return new OggVorbisAlbumArt(fref); + if (dynamic_cast(fref->file())) + return new OggFlacAlbumArt(fref); + + return nullptr; + } +}; + typedef struct _metadata_editor_s { FileRef *fref; IAlbumArt *ifart; @@ -503,18 +554,7 @@ extern "C" int metadata_editor_set_path(metadata_editor_h metadata, const char * return METADATA_EDITOR_ERROR_PERMISSION_DENIED; } - if (dynamic_cast(_metadata->fref->file())) - _metadata->ifart = new Mp3AlbumArt(_metadata->fref); - else if (dynamic_cast(_metadata->fref->file())) - _metadata->ifart = new Mp4AlbumArt(_metadata->fref); - else if (dynamic_cast(_metadata->fref->file())) - _metadata->ifart = new FlacAlbumArt(_metadata->fref); - else if (dynamic_cast(_metadata->fref->file())) - _metadata->ifart = new WavAlbumArt(_metadata->fref); - else if (dynamic_cast(_metadata->fref->file())) - _metadata->ifart = new OggVorbisAlbumArt(_metadata->fref); - else if (dynamic_cast(_metadata->fref->file())) - _metadata->ifart = new OggFlacAlbumArt(_metadata->fref); + _metadata->ifart = AlbumArtFactory::create(_metadata->fref); return METADATA_EDITOR_ERROR_NONE; } @@ -527,6 +567,7 @@ extern "C" int metadata_editor_append_picture(metadata_editor_h metadata, const ret = __check_file_validity(path); ME_RETVM_IF(ret != METADATA_EDITOR_ERROR_NONE, ret, "Invalid path"); + PictureFrame pic(path); ME_RETVM_IF(!pic.opened(), METADATA_EDITOR_ERROR_INVALID_PARAMETER, "Invalid path"); @@ -552,82 +593,59 @@ extern "C" int metadata_editor_get_picture(metadata_editor_h metadata, int index return _metadata->ifart->read(index, picture, size, mime_type); } -extern "C" int metadata_editor_set_metadata(metadata_editor_h metadata, metadata_editor_attr_e attribute, const char *value) +static const char * __get_attr_str(metadata_editor_attr_e attribute) { - auto _metadata = static_cast(metadata); - ME_RETV_IF(__is_valid_handle(_metadata, true), METADATA_EDITOR_ERROR_INVALID_PARAMETER); - switch (attribute) { case METADATA_EDITOR_ATTR_ARTIST: - return __set_to_property_map(_metadata->fref->file(), String("ARTIST"), value); + return "ARTIST"; case METADATA_EDITOR_ATTR_TITLE: - return __set_to_property_map(_metadata->fref->file(), String("TITLE"), value); + return "TITLE"; case METADATA_EDITOR_ATTR_ALBUM: - return __set_to_property_map(_metadata->fref->file(), String("ALBUM"), value); + return "ALBUM"; case METADATA_EDITOR_ATTR_GENRE: - return __set_to_property_map(_metadata->fref->file(), String("GENRE"), value); + return "GENRE"; case METADATA_EDITOR_ATTR_AUTHOR: - return __set_to_property_map(_metadata->fref->file(), String("COMPOSER"), value); + return "COMPOSER"; case METADATA_EDITOR_ATTR_COPYRIGHT: - return __set_to_property_map(_metadata->fref->file(), String("COPYRIGHT"), value); + return "COPYRIGHT"; case METADATA_EDITOR_ATTR_DATE: - return __set_to_property_map(_metadata->fref->file(), String("DATE"), value); + return "DATE"; case METADATA_EDITOR_ATTR_DESCRIPTION: - return __set_to_property_map(_metadata->fref->file(), String("DESCRIPTION"), value); + return "DESCRIPTION"; case METADATA_EDITOR_ATTR_COMMENT: - return __set_to_property_map(_metadata->fref->file(), String("COMMENT"), value); + return "COMMENT"; case METADATA_EDITOR_ATTR_TRACK_NUM: - return __set_to_property_map(_metadata->fref->file(), String("TRACKNUMBER"), value); + return "TRACKNUMBER"; case METADATA_EDITOR_ATTR_CONDUCTOR: - return __set_to_property_map(_metadata->fref->file(), String("CONDUCTOR"), value); + return "CONDUCTOR"; case METADATA_EDITOR_ATTR_UNSYNCLYRICS: - return __set_to_property_map(_metadata->fref->file(), String("LYRICS"), value); + return "LYRICS"; default: - ME_ERR("Invalid attribute"); - return METADATA_EDITOR_ERROR_INVALID_PARAMETER; + return ""; } } +extern "C" int metadata_editor_set_metadata(metadata_editor_h metadata, metadata_editor_attr_e attribute, const char *value) +{ + auto _metadata = static_cast(metadata); + ME_RETV_IF(__is_valid_handle(_metadata, true), METADATA_EDITOR_ERROR_INVALID_PARAMETER); + + return __set_to_property_map(_metadata->fref->file(), __get_attr_str(attribute), value); +} + extern "C" int metadata_editor_get_metadata(metadata_editor_h metadata, metadata_editor_attr_e attribute, char **value) { auto _metadata = static_cast(metadata); ME_RETV_IF(__is_valid_handle(_metadata, false), METADATA_EDITOR_ERROR_INVALID_PARAMETER); ME_RETVM_IF(!value, METADATA_EDITOR_ERROR_INVALID_PARAMETER, "Invalid value"); - PropertyMap tags = _metadata->fref->file()->properties(); - - switch (attribute) { - case METADATA_EDITOR_ATTR_ARTIST: - return __get_from_property_map(tags, String("ARTIST"), value); - case METADATA_EDITOR_ATTR_TITLE: - return __get_from_property_map(tags, String("TITLE"), value); - case METADATA_EDITOR_ATTR_ALBUM: - return __get_from_property_map(tags, String("ALBUM"), value); - case METADATA_EDITOR_ATTR_GENRE: - return __get_from_property_map(tags, String("GENRE"), value); - case METADATA_EDITOR_ATTR_AUTHOR: - return __get_from_property_map(tags, String("COMPOSER"), value); - case METADATA_EDITOR_ATTR_COPYRIGHT: - return __get_from_property_map(tags, String("COPYRIGHT"), value); - case METADATA_EDITOR_ATTR_DATE: - return __get_from_property_map(tags, String("DATE"), value); - case METADATA_EDITOR_ATTR_DESCRIPTION: - return __get_from_property_map(tags, String("DESCRIPTION"), value); - case METADATA_EDITOR_ATTR_COMMENT: - return __get_from_property_map(tags, String("COMMENT"), value); - case METADATA_EDITOR_ATTR_TRACK_NUM: - return __get_from_property_map(tags, String("TRACKNUMBER"), value); - case METADATA_EDITOR_ATTR_CONDUCTOR: - return __get_from_property_map(tags, String("CONDUCTOR"), value); - case METADATA_EDITOR_ATTR_UNSYNCLYRICS: - return __get_from_property_map(tags, String("LYRICS"), value); - case METADATA_EDITOR_ATTR_PICTURE_NUM: + /* exceptional case */ + if (attribute == METADATA_EDITOR_ATTR_PICTURE_NUM) { *value = g_strdup_printf("%u", _metadata->ifart->count()); return METADATA_EDITOR_ERROR_NONE; - default: - ME_ERR("Invalid attribute [%d]", attribute); - return METADATA_EDITOR_ERROR_INVALID_PARAMETER; } + + return __get_from_property_map(_metadata->fref->file()->properties(), __get_attr_str(attribute), value); } extern "C" int metadata_editor_update_metadata(metadata_editor_h metadata)