Re: [FFmpeg-devel] [PATCH 1/3] lavf/avio: Add new types to AVIODirEntryType, bump minor version
On Fri, Apr 03, 2015 at 03:54:08AM +0200, Mariusz Szczepańczyk wrote: --- doc/APIchanges| 5 + libavformat/avio.h| 5 - libavformat/version.h | 2 +- 3 files changed, 10 insertions(+), 2 deletions(-) applied thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The educated differ from the uneducated as much as the living from the dead. -- Aristotle signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] lavf/avio: Add new types to AVIODirEntryType, bump minor version
On 3 April 2015 at 16:05, Lukasz Marek lukasz.m.lu...@gmail.com wrote:
On 3 April 2015 at 15:22, Nicolas George geo...@nsup.org wrote:
Le quartidi 14 germinal, an CCXXIII, Mariusz Szczepańczyk a écrit :
diff --git a/libavformat/avio.h b/libavformat/avio.h
index f61a8f8..51913e3 100644
--- a/libavformat/avio.h
+++ b/libavformat/avio.h
@@ -63,7 +63,10 @@ enum AVIODirEntryType {
AVIO_ENTRY_NAMED_PIPE,
AVIO_ENTRY_SYMBOLIC_LINK,
AVIO_ENTRY_SOCKET,
-AVIO_ENTRY_FILE
+AVIO_ENTRY_FILE,
+AVIO_ENTRY_SERVER,
+AVIO_ENTRY_SHARE,
+AVIO_ENTRY_WORKGROUP,
};
Sorry if I missed the previous discussions on the mailing list (and if
not,
maybe just 8 hours before apply was a bit short for discussion), but I
had a
bit of a concern with this change.
Until know, if you wanted to make a recursive listing, you just had to
know
that you had to recurse into directories. Now... should you recurse into
shares? servers? workgroups? nobody knows.
There should be some way of knowing whether an entry can be opened like a
plain file, entered like a directory, or if it is just one of the weird
things that lay in some corners of filesystems, without requiring an
update
when new types are added.
You have good point about that, but regarding a patch itself do you see it
wrong?
Sever, share, workgroup could be mapped to directory, but what happen when
we have delete operation available in API. You can delete directory, but
how to delete a server or workgroup?
IMHO patch is OK.
AVIODirEntry can be extended with a flag can recurse to indicate that,
but still I believe author of the client application must be aware of what
protocol they are using and how it works.
Simple example with samba:
doc/examples/avio_list_dir smb:// will list workgroups
doc/examples/avio_list_dir smb://WORKGROUP will list servers in workgroup
doc/examples/avio_list_dir smb://SERVER will list shares on server
doc/examples/avio_list_dir smb://SERVER/SHARE will list files, dirs etc in
SHARE
smb://WORKGROUP/SERVER/SHARE is invalid.
So user must know how the protocol works. Samba is a bit specific case, but
stacking directories, openable entries is not enough.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] lavf/avio: Add new types to AVIODirEntryType, bump minor version
On Fri, Apr 3, 2015 at 4:52 PM, Lukasz Marek lukasz.m.lu...@gmail.com
wrote:
On 3 April 2015 at 16:05, Lukasz Marek lukasz.m.lu...@gmail.com wrote:
On 3 April 2015 at 15:22, Nicolas George geo...@nsup.org wrote:
Le quartidi 14 germinal, an CCXXIII, Mariusz Szczepańczyk a écrit :
diff --git a/libavformat/avio.h b/libavformat/avio.h
index f61a8f8..51913e3 100644
--- a/libavformat/avio.h
+++ b/libavformat/avio.h
@@ -63,7 +63,10 @@ enum AVIODirEntryType {
AVIO_ENTRY_NAMED_PIPE,
AVIO_ENTRY_SYMBOLIC_LINK,
AVIO_ENTRY_SOCKET,
-AVIO_ENTRY_FILE
+AVIO_ENTRY_FILE,
+AVIO_ENTRY_SERVER,
+AVIO_ENTRY_SHARE,
+AVIO_ENTRY_WORKGROUP,
};
Sorry if I missed the previous discussions on the mailing list (and if
not,
maybe just 8 hours before apply was a bit short for discussion), but I
had a
bit of a concern with this change.
Until know, if you wanted to make a recursive listing, you just had to
know
that you had to recurse into directories. Now... should you recurse into
shares? servers? workgroups? nobody knows.
There should be some way of knowing whether an entry can be opened like
a
plain file, entered like a directory, or if it is just one of the weird
things that lay in some corners of filesystems, without requiring an
update
when new types are added.
You have good point about that, but regarding a patch itself do you see
it
wrong?
Sever, share, workgroup could be mapped to directory, but what happen
when
we have delete operation available in API. You can delete directory, but
how to delete a server or workgroup?
IMHO patch is OK.
AVIODirEntry can be extended with a flag can recurse to indicate that,
but still I believe author of the client application must be aware of
what
protocol they are using and how it works.
Simple example with samba:
doc/examples/avio_list_dir smb:// will list workgroups
doc/examples/avio_list_dir smb://WORKGROUP will list servers in workgroup
doc/examples/avio_list_dir smb://SERVER will list shares on server
doc/examples/avio_list_dir smb://SERVER/SHARE will list files, dirs etc in
SHARE
smb://WORKGROUP/SERVER/SHARE is invalid.
So user must know how the protocol works. Samba is a bit specific case, but
stacking directories, openable entries is not enough.
I think there should be a separate function to construct a new url from a
url
and an entry object, that covers these cases or calls
av_append_path_component() for regular entries.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] lavf/avio: Add new types to AVIODirEntryType, bump minor version
Le quartidi 14 germinal, an CCXXIII, Mariusz Szczepańczyk a écrit : I think there should be a separate function to construct a new url from a url and an entry object, that covers these cases or calls av_append_path_component() for regular entries. Or we could just skip 20 years of API design and look at what POSIX did with openat(). POSIX forgot to separate open() from namei()/lookup, though, that is a shame. I do not know samba at all, though, so I can not say exactly how it maps. And to be clear: I had no objections to the patch itself. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] lavf/avio: Add new types to AVIODirEntryType, bump minor version
On 03.04.2015 16:19, Michael Niedermayer wrote:
On Fri, Apr 03, 2015 at 03:22:29PM +0200, Nicolas George wrote:
Le quartidi 14 germinal, an CCXXIII, Mariusz Szczepańczyk a écrit :
diff --git a/libavformat/avio.h b/libavformat/avio.h
index f61a8f8..51913e3 100644
--- a/libavformat/avio.h
+++ b/libavformat/avio.h
@@ -63,7 +63,10 @@ enum AVIODirEntryType {
AVIO_ENTRY_NAMED_PIPE,
AVIO_ENTRY_SYMBOLIC_LINK,
AVIO_ENTRY_SOCKET,
-AVIO_ENTRY_FILE
+AVIO_ENTRY_FILE,
+AVIO_ENTRY_SERVER,
+AVIO_ENTRY_SHARE,
+AVIO_ENTRY_WORKGROUP,
};
Sorry if I missed the previous discussions on the mailing list (and if not,
maybe just 8 hours before apply was a bit short for discussion), but I had a
bit of a concern with this change.
Until know, if you wanted to make a recursive listing, you just had to know
that you had to recurse into directories. Now... should you recurse into
shares? servers? workgroups? nobody knows.
There should be some way of knowing whether an entry can be opened like a
plain file, entered like a directory, or if it is just one of the weird
things that lay in some corners of filesystems, without requiring an update
when new types are added.
filemode could contain a bit for directories or a function could
provide this information, but for example links can be both file and
directory
or a new field could be added to AVIODirEntry
Fortunately this is not an issue for samba as it returns link's target
type. Mariusz has sent email to their mailing list with question about
this strange links treatment. But this have to be handled somehow for
next protocols. It cannot be a bit in filemode, because filemode may be
unknown and is set to -1 in this case.
I am not convinced that all enums that can be listed recursively has to
be marked explicitly. If thats just a case of symlink, then extending
struct with target's type and target's path may be more useful and also
solves the issue.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] lavf/avio: Add new types to AVIODirEntryType, bump minor version
On Fri, Apr 03, 2015 at 03:22:29PM +0200, Nicolas George wrote:
Le quartidi 14 germinal, an CCXXIII, Mariusz Szczepańczyk a écrit :
diff --git a/libavformat/avio.h b/libavformat/avio.h
index f61a8f8..51913e3 100644
--- a/libavformat/avio.h
+++ b/libavformat/avio.h
@@ -63,7 +63,10 @@ enum AVIODirEntryType {
AVIO_ENTRY_NAMED_PIPE,
AVIO_ENTRY_SYMBOLIC_LINK,
AVIO_ENTRY_SOCKET,
-AVIO_ENTRY_FILE
+AVIO_ENTRY_FILE,
+AVIO_ENTRY_SERVER,
+AVIO_ENTRY_SHARE,
+AVIO_ENTRY_WORKGROUP,
};
Sorry if I missed the previous discussions on the mailing list (and if not,
maybe just 8 hours before apply was a bit short for discussion), but I had a
bit of a concern with this change.
Until know, if you wanted to make a recursive listing, you just had to know
that you had to recurse into directories. Now... should you recurse into
shares? servers? workgroups? nobody knows.
There should be some way of knowing whether an entry can be opened like a
plain file, entered like a directory, or if it is just one of the weird
things that lay in some corners of filesystems, without requiring an update
when new types are added.
filemode could contain a bit for directories or a function could
provide this information, but for example links can be both file and
directory
or a new field could be added to AVIODirEntry
Is there a problem in the patch itself ?
should it be chnaged or reverted?
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.
signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] lavf/avio: Add new types to AVIODirEntryType, bump minor version
On 3 April 2015 at 15:22, Nicolas George geo...@nsup.org wrote:
Le quartidi 14 germinal, an CCXXIII, Mariusz Szczepańczyk a écrit :
diff --git a/libavformat/avio.h b/libavformat/avio.h
index f61a8f8..51913e3 100644
--- a/libavformat/avio.h
+++ b/libavformat/avio.h
@@ -63,7 +63,10 @@ enum AVIODirEntryType {
AVIO_ENTRY_NAMED_PIPE,
AVIO_ENTRY_SYMBOLIC_LINK,
AVIO_ENTRY_SOCKET,
-AVIO_ENTRY_FILE
+AVIO_ENTRY_FILE,
+AVIO_ENTRY_SERVER,
+AVIO_ENTRY_SHARE,
+AVIO_ENTRY_WORKGROUP,
};
Sorry if I missed the previous discussions on the mailing list (and if not,
maybe just 8 hours before apply was a bit short for discussion), but I had
a
bit of a concern with this change.
Until know, if you wanted to make a recursive listing, you just had to know
that you had to recurse into directories. Now... should you recurse into
shares? servers? workgroups? nobody knows.
There should be some way of knowing whether an entry can be opened like a
plain file, entered like a directory, or if it is just one of the weird
things that lay in some corners of filesystems, without requiring an update
when new types are added.
You have good point about that, but regarding a patch itself do you see it
wrong?
Sever, share, workgroup could be mapped to directory, but what happen when
we have delete operation available in API. You can delete directory, but
how to delete a server or workgroup?
IMHO patch is OK.
AVIODirEntry can be extended with a flag can recurse to indicate that,
but still I believe author of the client application must be aware of what
protocol they are using and how it works.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] lavf/avio: Add new types to AVIODirEntryType, bump minor version
Le quartidi 14 germinal, an CCXXIII, Mariusz Szczepańczyk a écrit :
diff --git a/libavformat/avio.h b/libavformat/avio.h
index f61a8f8..51913e3 100644
--- a/libavformat/avio.h
+++ b/libavformat/avio.h
@@ -63,7 +63,10 @@ enum AVIODirEntryType {
AVIO_ENTRY_NAMED_PIPE,
AVIO_ENTRY_SYMBOLIC_LINK,
AVIO_ENTRY_SOCKET,
-AVIO_ENTRY_FILE
+AVIO_ENTRY_FILE,
+AVIO_ENTRY_SERVER,
+AVIO_ENTRY_SHARE,
+AVIO_ENTRY_WORKGROUP,
};
Sorry if I missed the previous discussions on the mailing list (and if not,
maybe just 8 hours before apply was a bit short for discussion), but I had a
bit of a concern with this change.
Until know, if you wanted to make a recursive listing, you just had to know
that you had to recurse into directories. Now... should you recurse into
shares? servers? workgroups? nobody knows.
There should be some way of knowing whether an entry can be opened like a
plain file, entered like a directory, or if it is just one of the weird
things that lay in some corners of filesystems, without requiring an update
when new types are added.
Regards,
--
Nicolas George
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] lavf/avio: Add new types to AVIODirEntryType, bump minor version
On Fri, Apr 3, 2015 at 3:22 PM, Nicolas George geo...@nsup.org wrote:
Le quartidi 14 germinal, an CCXXIII, Mariusz Szczepańczyk a écrit :
diff --git a/libavformat/avio.h b/libavformat/avio.h
index f61a8f8..51913e3 100644
--- a/libavformat/avio.h
+++ b/libavformat/avio.h
@@ -63,7 +63,10 @@ enum AVIODirEntryType {
AVIO_ENTRY_NAMED_PIPE,
AVIO_ENTRY_SYMBOLIC_LINK,
AVIO_ENTRY_SOCKET,
-AVIO_ENTRY_FILE
+AVIO_ENTRY_FILE,
+AVIO_ENTRY_SERVER,
+AVIO_ENTRY_SHARE,
+AVIO_ENTRY_WORKGROUP,
};
Sorry if I missed the previous discussions on the mailing list (and if not,
maybe just 8 hours before apply was a bit short for discussion), but I had
a
bit of a concern with this change.
Until know, if you wanted to make a recursive listing, you just had to know
that you had to recurse into directories. Now... should you recurse into
shares? servers? workgroups? nobody knows.
There should be some way of knowing whether an entry can be opened like a
plain file, entered like a directory, or if it is just one of the weird
things that lay in some corners of filesystems, without requiring an update
when new types are added.
Right. Even though user should at least roughly be aware of what specific
type is representing, the api should provide a way to check if an entry is
readable as file or listable as directory.
I see two viable solutions: adding new flags to AVIODirEntry or adding
functions like:
bool avio_is_listable(AVIODirEntry *entry);
bool avio_is_readable(AVIODirEntry *entry);
(never mind the names, just example)
I tend to prefer the second option because these properties should
be completely dependant on type, and if it turns out they're not, the
implementation can easily be changed/expanded.
Regards,
Mariusz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
